Attachment 341322 Details for Bug 185931 – patch

[patch] patch

a-backup.diff (text/plain), 7.84 KB, created by Saam Barati on 2018-05-25 13:56:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-05-25 13:56:54 PDT

Size: 7.84 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 232197)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,32 @@
>+2018-05-25  Saam Barati  <sbarati@apple.com>
>+
>+        Make JSC have a mini mode that kicks in when the JIT is disabled
>+        https://bugs.webkit.org/show_bug.cgi?id=185931
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch makes JSC have a mini VM mode. This currently only kicks in
>+        when the process can't JIT. Mini VM now means two thing:
>+        - We always use a 1.27x heap growth factor.
>+        - We always sweep synchronously.
>+        
>+        I'm going to continue to extend what mini VM mode means.
>+        
>+        This patch is a 50% memory progression on and a ~8-9% time regression
>+        on run-testmem when running in mini VM mode.
>+
>+        * heap/Heap.cpp:
>+        (JSC::Heap::collectNow):
>+        (JSC::Heap::finalize):
>+        (JSC::Heap::useGenerationalGC):
>+        (JSC::Heap::shouldSweepSynchronously):
>+        (JSC::Heap::shouldDoFullCollection):
>+        * heap/Heap.h:
>+        * runtime/Options.h:
>+        * runtime/VM.cpp:
>+        (JSC::VM::isInMiniMode):
>+        * runtime/VM.h:
>+
> 2018-05-25  Saam Barati  <sbarati@apple.com>
> 
>         Have a memory test where we can validate JSCs mini memory mode
>Index: Source/JavaScriptCore/heap/Heap.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/Heap.cpp	(revision 232197)
>+++ Source/JavaScriptCore/heap/Heap.cpp	(working copy)
>@@ -127,6 +127,9 @@ size_t minHeapSize(HeapType heapType, si
> 
> size_t proportionalHeapSize(size_t heapSize, size_t ramSize)
> {
>+    if (VM::isInMiniMode())
>+        return Options::miniVMHeapGrowthFactor() * heapSize;
>+
> #if PLATFORM(IOS)
>     size_t memoryFootprint = bmalloc::api::memoryFootprint();
>     if (memoryFootprint < ramSize * Options::smallHeapRAMFraction())
>@@ -1049,7 +1052,7 @@ void Heap::collectNow(Synchronousness sy
>         if (UNLIKELY(Options::useImmortalObjects()))
>             sweeper().stopSweeping();
>         
>-        bool alreadySweptInCollectSync = Options::sweepSynchronously();
>+        bool alreadySweptInCollectSync = shouldSweepSynchronously();
>         if (!alreadySweptInCollectSync) {
>             if (Options::logGC())
>                 dataLog("[GC<", RawPointer(this), ">: ");
>@@ -2043,7 +2046,7 @@ void Heap::finalize()
>     for (const HeapFinalizerCallback& callback : m_heapFinalizerCallbacks)
>         callback.run(*vm());
>     
>-    if (Options::sweepSynchronously())
>+    if (shouldSweepSynchronously())
>         sweepSynchronously();
> 
>     if (Options::logGC()) {
>@@ -2396,9 +2399,19 @@ void Heap::collectNowFullIfNotDoneRecent
>     collectNow(synchronousness, CollectionScope::Full);
> }
> 
>+bool Heap::useGenerationalGC()
>+{
>+    return Options::useGenerationalGC() && !VM::isInMiniMode();
>+}
>+
>+bool Heap::shouldSweepSynchronously()
>+{
>+    return Options::sweepSynchronously() || VM::isInMiniMode();
>+}
>+
> bool Heap::shouldDoFullCollection()
> {
>-    if (!Options::useGenerationalGC())
>+    if (!useGenerationalGC())
>         return true;
> 
>     if (!m_currentRequest.scope)
>Index: Source/JavaScriptCore/heap/Heap.h
>===================================================================
>--- Source/JavaScriptCore/heap/Heap.h	(revision 232197)
>+++ Source/JavaScriptCore/heap/Heap.h	(working copy)
>@@ -556,6 +556,9 @@ private:
>     void assertMarkStacksEmpty();
> 
>     void setBonusVisitorTask(RefPtr<SharedTask<void(SlotVisitor&)>>);
>+
>+    static bool useGenerationalGC();
>+    static bool shouldSweepSynchronously();
>     
>     const HeapType m_heapType;
>     const size_t m_ramSize;
>Index: Source/JavaScriptCore/runtime/Options.h
>===================================================================
>--- Source/JavaScriptCore/runtime/Options.h	(revision 232197)
>+++ Source/JavaScriptCore/runtime/Options.h	(working copy)
>@@ -237,6 +237,7 @@ constexpr bool enableWebAssemblyStreamin
>     v(double, mediumHeapRAMFraction, 0.5, Normal, nullptr) \
>     v(double, mediumHeapGrowthFactor, 1.5, Normal, nullptr) \
>     v(double, largeHeapGrowthFactor, 1.24, Normal, nullptr) \
>+    v(double, miniVMHeapGrowthFactor, 1.27, Normal, nullptr) \
>     v(double, criticalGCMemoryThreshold, 0.80, Normal, "percent memory in use the GC considers critical.  The collector is much more aggressive above this threshold") \
>     v(double, minimumMutatorUtilization, 0, Normal, nullptr) \
>     v(double, maximumMutatorUtilization, 0.7, Normal, nullptr) \
>@@ -509,8 +510,9 @@ constexpr bool enableWebAssemblyStreamin
>     v(bool, useBigInt, false, Normal, "If true, we will enable BigInt support.") \
>     v(bool, useIntlNumberFormatToParts, enableIntlNumberFormatToParts, Normal, "If true, we will enable Intl.NumberFormat.prototype.formatToParts") \
>     v(bool, useIntlPluralRules, enableIntlPluralRules, Normal, "If true, we will enable Intl.PluralRules.") \
>-    v(bool, useArrayAllocationProfiling, true, Normal, "If true, we will use our normal array allocation profiling. If false, the allocation profile will always claim to be undecided.")\
>-    v(bool, forcePolyProto, false, Normal, "If true, create_this will always create an object with a poly proto structure.")
>+    v(bool, useArrayAllocationProfiling, true, Normal, "If true, we will use our normal array allocation profiling. If false, the allocation profile will always claim to be undecided.") \
>+    v(bool, forcePolyProto, false, Normal, "If true, create_this will always create an object with a poly proto structure.") \
>+    v(bool, forceMiniVMMode, false, Normal, "If true, it will force mini VM mode on.")
> 
> 
> enum OptionEquivalence {
>Index: Source/JavaScriptCore/runtime/VM.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.cpp	(revision 232197)
>+++ Source/JavaScriptCore/runtime/VM.cpp	(working copy)
>@@ -238,6 +238,11 @@ bool VM::canUseRegExpJIT()
> #endif
> }
> 
>+bool VM::isInMiniMode()
>+{
>+    return !canUseJIT() || Options::forceMiniVMMode();
>+}
>+
> VM::VM(VMType vmType, HeapType heapType)
>     : m_apiLock(adoptRef(new JSLock(this)))
> #if USE(CF)
>Index: Source/JavaScriptCore/runtime/VM.h
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.h	(revision 232197)
>+++ Source/JavaScriptCore/runtime/VM.h	(working copy)
>@@ -558,6 +558,7 @@ public:
>     static JS_EXPORT_PRIVATE bool canUseAssembler();
>     static JS_EXPORT_PRIVATE bool canUseJIT();
>     static JS_EXPORT_PRIVATE bool canUseRegExpJIT();
>+    static JS_EXPORT_PRIVATE bool isInMiniMode();
> 
>     SourceProviderCache* addSourceProviderCache(SourceProvider*);
>     void clearSourceProviderCaches();
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 232198)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2018-05-25  Saam Barati  <sbarati@apple.com>
>+
>+        Make JSC have a mini mode that kicks in when the JIT is disabled
>+        https://bugs.webkit.org/show_bug.cgi?id=185931
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This renames a variable for clarity.
>+
>+        * Scripts/run-testmem:
>+
> 2018-05-25  Alex Christensen  <achristensen@webkit.org>
> 
>         URL::host should return a StringView to reduce allocations
>Index: Tools/Scripts/run-testmem
>===================================================================
>--- Tools/Scripts/run-testmem	(revision 232197)
>+++ Tools/Scripts/run-testmem	(working copy)
>@@ -121,14 +121,14 @@ def getTests
>     files.sort_by { | (path) | File.basename(path) }
> end
> 
>-def geomean(x)
>-    score = x.inject(1.0, :*)
>-    score ** (1.0 / x.length)
>+def geomean(arr)
>+    score = arr.inject(1.0, :*)
>+    score ** (1.0 / arr.length)
> end
> 
>-def mean(x)
>-    sum = x.inject(0.0, :+)
>-    sum / x.length
>+def mean(arr)
>+    sum = arr.inject(0.0, :+)
>+    sum / arr.length
> end
> 
> def runTest(path, iters)

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185931: 341322