[webkit-changes] [236296] trunk/Source/JavaScriptCore

Thu, 20 Sep 2018 18:12:51 -0700

Title: [236296] trunk/Source/_javascript_Core
Revision
236296
Author
yusukesuz...@slowstart.org
Date
2018-09-20 18:11:19 -0700 (Thu, 20 Sep 2018)

Log Message

[JSC] Heap::reportExtraMemoryVisited shows contention if we have many JSString
https://bugs.webkit.org/show_bug.cgi?id=189558

Reviewed by Mark Lam.

When running web-tooling-benchmark postcss test on Linux JSCOnly port, we get the following result in `perf report`.

    10.95%  AutomaticThread  libJavaScriptCore.so.1.0.0  [.] JSC::Heap::reportExtraMemoryVisited

This is because postcss produces bunch of JSString, which require reportExtraMemoryVisited calls in JSString::visitChildren.
And since reportExtraMemoryVisited attempts to update atomic counter, if we have bunch of marking threads, it becomes super contended.

This patch reduces the frequency of updating the atomic counter. Each SlotVisitor has per-SlotVisitor m_extraMemorySize counter.
And we propagate this value to the global atomic counter when rebalance happens.

We also reduce HeapCell::heap() access by using `vm.heap`.

* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::didStartMarking):
(JSC::SlotVisitor::propagateExternalMemoryVisitedIfNecessary):
(JSC::SlotVisitor::drain):
(JSC::SlotVisitor::performIncrementOfDraining):
* heap/SlotVisitor.h:
* heap/SlotVisitorInlines.h:
(JSC::SlotVisitor::reportExtraMemoryVisited):
* runtime/JSString.cpp:
(JSC::JSRopeString::resolveRopeToAtomicString const):
(JSC::JSRopeString::resolveRope const):
* runtime/JSString.h:
(JSC::JSString::finishCreation):
* wasm/js/JSWebAssemblyInstance.cpp:
(JSC::JSWebAssemblyInstance::finishCreation):
* wasm/js/JSWebAssemblyMemory.cpp:
(JSC::JSWebAssemblyMemory::finishCreation):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (236295 => 236296)


--- trunk/Source/_javascript_Core/ChangeLog	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-09-21 01:11:19 UTC (rev 236296)
@@ -1,3 +1,40 @@
+2018-09-20  Yusuke Suzuki  <yusukesuz...@slowstart.org>
+
+        [JSC] Heap::reportExtraMemoryVisited shows contention if we have many JSString
+        https://bugs.webkit.org/show_bug.cgi?id=189558
+
+        Reviewed by Mark Lam.
+
+        When running web-tooling-benchmark postcss test on Linux JSCOnly port, we get the following result in `perf report`.
+
+            10.95%  AutomaticThread  libJavaScriptCore.so.1.0.0  [.] JSC::Heap::reportExtraMemoryVisited
+
+        This is because postcss produces bunch of JSString, which require reportExtraMemoryVisited calls in JSString::visitChildren.
+        And since reportExtraMemoryVisited attempts to update atomic counter, if we have bunch of marking threads, it becomes super contended.
+
+        This patch reduces the frequency of updating the atomic counter. Each SlotVisitor has per-SlotVisitor m_extraMemorySize counter.
+        And we propagate this value to the global atomic counter when rebalance happens.
+
+        We also reduce HeapCell::heap() access by using `vm.heap`.
+
+        * heap/SlotVisitor.cpp:
+        (JSC::SlotVisitor::didStartMarking):
+        (JSC::SlotVisitor::propagateExternalMemoryVisitedIfNecessary):
+        (JSC::SlotVisitor::drain):
+        (JSC::SlotVisitor::performIncrementOfDraining):
+        * heap/SlotVisitor.h:
+        * heap/SlotVisitorInlines.h:
+        (JSC::SlotVisitor::reportExtraMemoryVisited):
+        * runtime/JSString.cpp:
+        (JSC::JSRopeString::resolveRopeToAtomicString const):
+        (JSC::JSRopeString::resolveRope const):
+        * runtime/JSString.h:
+        (JSC::JSString::finishCreation):
+        * wasm/js/JSWebAssemblyInstance.cpp:
+        (JSC::JSWebAssemblyInstance::finishCreation):
+        * wasm/js/JSWebAssemblyMemory.cpp:
+        (JSC::JSWebAssemblyMemory::finishCreation):
+
 2018-09-20  Michael Saboff  <msab...@apple.com>
 
         Add functions to measure memory footprint to JSC

Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (236295 => 236296)


--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp	2018-09-21 01:11:19 UTC (rev 236296)
@@ -99,8 +99,17 @@
 
 void SlotVisitor::didStartMarking()
 {
-    if (heap()->collectionScope() == CollectionScope::Eden)
-        reset();
+    auto scope = heap()->collectionScope();
+    if (scope) {
+        switch (*scope) {
+        case CollectionScope::Eden:
+            reset();
+            break;
+        case CollectionScope::Full:
+            m_extraMemorySize = 0;
+            break;
+        }
+    }
 
     if (HeapProfiler* heapProfiler = vm().heapProfiler())
         m_heapSnapshotBuilder = heapProfiler->activeSnapshotBuilder();
@@ -397,6 +406,17 @@
     visitChildren(cell);
 }
 
+inline void SlotVisitor::propagateExternalMemoryVisitedIfNecessary()
+{
+    if (m_isFirstVisit) {
+        if (m_extraMemorySize.hasOverflowed())
+            heap()->reportExtraMemoryVisited(std::numeric_limits<size_t>::max());
+        else if (m_extraMemorySize)
+            heap()->reportExtraMemoryVisited(m_extraMemorySize.unsafeGet());
+        m_extraMemorySize = 0;
+    }
+}
+
 void SlotVisitor::donateKnownParallel(MarkStackArray& from, MarkStackArray& to)
 {
     // NOTE: Because we re-try often, we can afford to be conservative, and
@@ -483,6 +503,7 @@
                     visitChildren(stack.removeLast());
                 return IterationStatus::Done;
             });
+        propagateExternalMemoryVisitedIfNecessary();
         if (status == IterationStatus::Continue)
             break;
         
@@ -539,6 +560,7 @@
                     }
                     return IterationStatus::Done;
                 });
+            propagateExternalMemoryVisitedIfNecessary();
             if (status == IterationStatus::Continue)
                 break;
             m_rightToRun.safepoint();

Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (236295 => 236296)


--- trunk/Source/_javascript_Core/heap/SlotVisitor.h	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h	2018-09-21 01:11:19 UTC (rev 236296)
@@ -216,6 +216,8 @@
     void noteLiveAuxiliaryCell(HeapCell*);
     
     void visitChildren(const JSCell*);
+
+    void propagateExternalMemoryVisitedIfNecessary();
     
     void donateKnownParallel();
     void donateKnownParallel(MarkStackArray& from, MarkStackArray& to);
@@ -237,6 +239,7 @@
     size_t m_bytesVisited;
     size_t m_visitCount;
     size_t m_nonCellVisitCount { 0 }; // Used for incremental draining, ignored otherwise.
+    Checked<size_t, RecordOverflow> m_extraMemorySize { 0 };
     bool m_isInParallelMode;
 
     HeapVersion m_markingVersion;

Modified: trunk/Source/_javascript_Core/heap/SlotVisitorInlines.h (236295 => 236296)


--- trunk/Source/_javascript_Core/heap/SlotVisitorInlines.h	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/heap/SlotVisitorInlines.h	2018-09-21 01:11:19 UTC (rev 236296)
@@ -154,8 +154,10 @@
 inline void SlotVisitor::reportExtraMemoryVisited(size_t size)
 {
     if (m_isFirstVisit) {
-        heap()->reportExtraMemoryVisited(size);
         m_nonCellVisitCount += size;
+        // FIXME: Change this to use SaturatedArithmetic when available.
+        // https://bugs.webkit.org/show_bug.cgi?id=170411
+        m_extraMemorySize += size;
     }
 }

Modified: trunk/Source/_javascript_Core/runtime/JSString.cpp (236295 => 236296)


--- trunk/Source/_javascript_Core/runtime/JSString.cpp	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/runtime/JSString.cpp	2018-09-21 01:11:19 UTC (rev 236296)
@@ -205,7 +205,7 @@
 
     // If we resolved a string that didn't previously exist, notify the heap that we've grown.
     if (m_value.impl()->hasOneRef())
-        Heap::heap(this)->reportExtraMemoryAllocated(m_value.impl()->cost());
+        vm.heap.reportExtraMemoryAllocated(m_value.impl()->cost());
 }
 
 void JSRopeString::clearFibers() const
@@ -264,7 +264,7 @@
     if (is8Bit()) {
         LChar* buffer;
         if (auto newImpl = StringImpl::tryCreateUninitialized(length(), buffer)) {
-            Heap::heap(this)->reportExtraMemoryAllocated(newImpl->cost());
+            exec->vm().heap.reportExtraMemoryAllocated(newImpl->cost());
             m_value = WTFMove(newImpl);
         } else {
             outOfMemory(exec);
@@ -278,7 +278,7 @@
     
     UChar* buffer;
     if (auto newImpl = StringImpl::tryCreateUninitialized(length(), buffer)) {
-        Heap::heap(this)->reportExtraMemoryAllocated(newImpl->cost());
+        exec->vm().heap.reportExtraMemoryAllocated(newImpl->cost());
         m_value = WTFMove(newImpl);
     } else {
         outOfMemory(exec);

Modified: trunk/Source/_javascript_Core/runtime/JSString.h (236295 => 236296)


--- trunk/Source/_javascript_Core/runtime/JSString.h	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/runtime/JSString.h	2018-09-21 01:11:19 UTC (rev 236296)
@@ -123,7 +123,7 @@
         Base::finishCreation(vm);
         setLength(length);
         setIs8Bit(m_value.impl()->is8Bit());
-        Heap::heap(this)->reportExtraMemoryAllocated(cost);
+        vm.heap.reportExtraMemoryAllocated(cost);
     }
 
 protected:

Modified: trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyInstance.cpp (236295 => 236296)


--- trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyInstance.cpp	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyInstance.cpp	2018-09-21 01:11:19 UTC (rev 236296)
@@ -66,7 +66,7 @@
     m_moduleNamespaceObject.set(vm, this, moduleNamespaceObject);
     m_callee.set(vm, this, module->callee());
 
-    heap()->reportExtraMemoryAllocated(m_instance->extraMemoryAllocated());
+    vm.heap.reportExtraMemoryAllocated(m_instance->extraMemoryAllocated());
 }
 
 void JSWebAssemblyInstance::destroy(JSCell* cell)

Modified: trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyMemory.cpp (236295 => 236296)


--- trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyMemory.cpp	2018-09-21 00:28:23 UTC (rev 236295)
+++ trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyMemory.cpp	2018-09-21 01:11:19 UTC (rev 236296)
@@ -133,7 +133,7 @@
 {
     Base::finishCreation(vm);
     ASSERT(inherits(vm, info()));
-    heap()->reportExtraMemoryAllocated(memory().size());
+    vm.heap.reportExtraMemoryAllocated(memory().size());
 }
 
 void JSWebAssemblyMemory::destroy(JSCell* cell)

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to