<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[209766] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/209766">209766</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-12-13 11:54:15 -0800 (Tue, 13 Dec 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Make opaque root scanning truly constraint-based
https://bugs.webkit.org/show_bug.cgi?id=165760
Reviewed by Saam Barati.
Source/JavaScriptCore:
We have bugs when visitChildren() changes its mind about what opaque root to add, since
we don't have barriers on opaque roots. This supposedly once worked for generational GC,
and I started adding more barriers to support concurrent GC. But I think that the real
bug here is that we want the JSObject->OpaqueRoot to be evaluated as a constraint that
participates in the fixpoint. A constraint is different from the normal visiting in that
the GC will not wait for a barrier to rescan the object.
So, it's now possible for any visitChildren() method to become a constraint by calling
slotVisitor.rescanAsConstraint(). Because opaque roots are constraints, addOpaqueRoot()
does rescanAsConstraint() for you.
The constraint set is simply a HashSet<JSCell*> that accumulates with every
rescanAsConstraint() call and is only cleared at the start of full GC. This trivially
resolves most classes of GC bugs that would have arisen from opaque roots being changed
in a way that the GC did not anticipate.
Looks like this is perf-neutral.
* heap/Heap.cpp:
(JSC::Heap::markToFixpoint):
(JSC::Heap::setMutatorShouldBeFenced):
(JSC::Heap::writeBarrierOpaqueRootSlow): Deleted.
(JSC::Heap::addMutatorShouldBeFencedCache): Deleted.
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::writeBarrierOpaqueRoot): Deleted.
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::visitWeakSets):
* heap/MarkedSpace.h:
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::visitChildren):
(JSC::SlotVisitor::visitSubsequently):
(JSC::SlotVisitor::drain):
(JSC::SlotVisitor::addOpaqueRoot):
(JSC::SlotVisitor::rescanAsConstraint):
(JSC::SlotVisitor::mergeIfNecessary):
(JSC::SlotVisitor::mergeOpaqueRootsAndConstraints):
(JSC::SlotVisitor::mergeOpaqueRootsIfNecessary): Deleted.
* heap/SlotVisitor.h:
* heap/SlotVisitorInlines.h:
(JSC::SlotVisitor::reportExtraMemoryVisited):
(JSC::SlotVisitor::reportExternalMemoryVisited):
(JSC::SlotVisitor::didNotRace):
* heap/WeakBlock.cpp:
(JSC::WeakBlock::specializedVisit):
(JSC::WeakBlock::visit):
* heap/WeakBlock.h:
* heap/WeakSet.h:
(JSC::WeakSet::visit):
Source/WebCore:
No new tests yet. I think that writing tests for this is a big investigation:
https://bugs.webkit.org/show_bug.cgi?id=165808
Remove the previous advancing wavefront DOM write barrier. I don't think this will scale
very well. It's super confusing.
This change makes it so that visitChildren can become a GC constraint that executes as
part of the fixpoint. This changes all WebCore visitChildren methods that do opaque
roots into constraints.
* bindings/js/CommonVM.cpp:
(WebCore::commonVMSlow):
(WebCore::writeBarrierOpaqueRootSlow): Deleted.
* bindings/js/CommonVM.h:
(WebCore::writeBarrierOpaqueRoot): Deleted.
* bindings/js/JSAttrCustom.cpp:
(WebCore::JSAttr::visitAdditionalChildren):
* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::visitAdditionalChildren):
* bindings/js/JSIDBCursorCustom.cpp:
(WebCore::JSIDBCursor::visitAdditionalChildren):
* bindings/js/JSMessageChannelCustom.cpp:
(WebCore::JSMessageChannel::visitAdditionalChildren):
* bindings/js/JSMessagePortCustom.cpp:
(WebCore::JSMessagePort::visitAdditionalChildren):
* bindings/js/JSNodeIteratorCustom.cpp:
(WebCore::JSNodeIterator::visitAdditionalChildren):
* bindings/js/JSTextTrackCueCustom.cpp:
(WebCore::JSTextTrackCue::visitAdditionalChildren):
* bindings/js/JSTreeWalkerCustom.cpp:
(WebCore::JSTreeWalker::visitAdditionalChildren):
* bindings/js/JSWorkerGlobalScopeCustom.cpp:
(WebCore::JSWorkerGlobalScope::visitAdditionalChildren):
* bindings/js/JSXMLHttpRequestCustom.cpp:
(WebCore::JSXMLHttpRequest::visitAdditionalChildren):
* bindings/js/JSXPathResultCustom.cpp:
(WebCore::JSXPathResult::visitAdditionalChildren):
* dom/ContainerNodeAlgorithms.cpp:
(WebCore::notifyChildNodeInserted):
(WebCore::notifyChildNodeRemoved):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeaph">trunk/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapInlinesh">trunk/Source/JavaScriptCore/heap/HeapInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkedSpacecpp">trunk/Source/JavaScriptCore/heap/MarkedSpace.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkedSpaceh">trunk/Source/JavaScriptCore/heap/MarkedSpace.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorcpp">trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorh">trunk/Source/JavaScriptCore/heap/SlotVisitor.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorInlinesh">trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapWeakBlockcpp">trunk/Source/JavaScriptCore/heap/WeakBlock.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapWeakBlockh">trunk/Source/JavaScriptCore/heap/WeakBlock.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapWeakSeth">trunk/Source/JavaScriptCore/heap/WeakSet.h</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsjsCommonVMcpp">trunk/Source/WebCore/bindings/js/CommonVM.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsCommonVMh">trunk/Source/WebCore/bindings/js/CommonVM.h</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSAttrCustomcpp">trunk/Source/WebCore/bindings/js/JSAttrCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSDOMWindowCustomcpp">trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSIDBCursorCustomcpp">trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSMessageChannelCustomcpp">trunk/Source/WebCore/bindings/js/JSMessageChannelCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSMessagePortCustomcpp">trunk/Source/WebCore/bindings/js/JSMessagePortCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSNodeIteratorCustomcpp">trunk/Source/WebCore/bindings/js/JSNodeIteratorCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSTextTrackCueCustomcpp">trunk/Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSTreeWalkerCustomcpp">trunk/Source/WebCore/bindings/js/JSTreeWalkerCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSWorkerGlobalScopeCustomcpp">trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSXMLHttpRequestCustomcpp">trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSXPathResultCustomcpp">trunk/Source/WebCore/bindings/js/JSXPathResultCustom.cpp</a></li>
<li><a href="#trunkSourceWebCoredomContainerNodeAlgorithmscpp">trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -1,3 +1,60 @@
</span><ins>+2016-12-13 Filip Pizlo <fpizlo@apple.com>
+
+ Make opaque root scanning truly constraint-based
+ https://bugs.webkit.org/show_bug.cgi?id=165760
+
+ Reviewed by Saam Barati.
+
+ We have bugs when visitChildren() changes its mind about what opaque root to add, since
+ we don't have barriers on opaque roots. This supposedly once worked for generational GC,
+ and I started adding more barriers to support concurrent GC. But I think that the real
+ bug here is that we want the JSObject->OpaqueRoot to be evaluated as a constraint that
+ participates in the fixpoint. A constraint is different from the normal visiting in that
+ the GC will not wait for a barrier to rescan the object.
+
+ So, it's now possible for any visitChildren() method to become a constraint by calling
+ slotVisitor.rescanAsConstraint(). Because opaque roots are constraints, addOpaqueRoot()
+ does rescanAsConstraint() for you.
+
+ The constraint set is simply a HashSet<JSCell*> that accumulates with every
+ rescanAsConstraint() call and is only cleared at the start of full GC. This trivially
+ resolves most classes of GC bugs that would have arisen from opaque roots being changed
+ in a way that the GC did not anticipate.
+
+ Looks like this is perf-neutral.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::markToFixpoint):
+ (JSC::Heap::setMutatorShouldBeFenced):
+ (JSC::Heap::writeBarrierOpaqueRootSlow): Deleted.
+ (JSC::Heap::addMutatorShouldBeFencedCache): Deleted.
+ * heap/Heap.h:
+ * heap/HeapInlines.h:
+ (JSC::Heap::writeBarrierOpaqueRoot): Deleted.
+ * heap/MarkedSpace.cpp:
+ (JSC::MarkedSpace::visitWeakSets):
+ * heap/MarkedSpace.h:
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::visitChildren):
+ (JSC::SlotVisitor::visitSubsequently):
+ (JSC::SlotVisitor::drain):
+ (JSC::SlotVisitor::addOpaqueRoot):
+ (JSC::SlotVisitor::rescanAsConstraint):
+ (JSC::SlotVisitor::mergeIfNecessary):
+ (JSC::SlotVisitor::mergeOpaqueRootsAndConstraints):
+ (JSC::SlotVisitor::mergeOpaqueRootsIfNecessary): Deleted.
+ * heap/SlotVisitor.h:
+ * heap/SlotVisitorInlines.h:
+ (JSC::SlotVisitor::reportExtraMemoryVisited):
+ (JSC::SlotVisitor::reportExternalMemoryVisited):
+ (JSC::SlotVisitor::didNotRace):
+ * heap/WeakBlock.cpp:
+ (JSC::WeakBlock::specializedVisit):
+ (JSC::WeakBlock::visit):
+ * heap/WeakBlock.h:
+ * heap/WeakSet.h:
+ (JSC::WeakSet::visit):
+
</ins><span class="cx"> 2016-12-13 Commit Queue <commit-queue@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r209725.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -519,6 +519,7 @@
</span><span class="cx">
</span><span class="cx"> if (m_collectionScope == CollectionScope::Full) {
</span><span class="cx"> m_opaqueRoots.clear();
</span><ins>+ m_constraints.clear();
</ins><span class="cx"> m_collectorSlotVisitor->clearMarkStacks();
</span><span class="cx"> m_mutatorMarkStack->clear();
</span><span class="cx"> }
</span><span class="lines">@@ -617,11 +618,15 @@
</span><span class="cx">
</span><span class="cx"> m_jitStubRoutines->traceMarkedStubRoutines(*m_collectorSlotVisitor);
</span><span class="cx">
</span><del>- m_collectorSlotVisitor->mergeOpaqueRootsIfNecessary();
</del><ins>+ m_collectorSlotVisitor->mergeIfNecessary();
</ins><span class="cx"> for (auto& parallelVisitor : m_parallelSlotVisitors)
</span><del>- parallelVisitor->mergeOpaqueRootsIfNecessary();
</del><ins>+ parallelVisitor->mergeIfNecessary();
+
+ for (JSCell* cell : m_constraints)
+ m_collectorSlotVisitor->visitSubsequently(cell);
+ m_collectorSlotVisitor->mergeIfNecessary();
</ins><span class="cx">
</span><del>- m_objectSpace.visitWeakSets(heapRootVisitor);
</del><ins>+ size_t weakSetCount = m_objectSpace.visitWeakSets(heapRootVisitor);
</ins><span class="cx"> harvestWeakReferences();
</span><span class="cx"> visitCompilerWorklistWeakReferences();
</span><span class="cx"> DFG::markCodeBlocks(*m_vm, *m_collectorSlotVisitor);
</span><span class="lines">@@ -628,7 +633,7 @@
</span><span class="cx"> bool shouldTerminate = m_collectorSlotVisitor->isEmpty() && m_mutatorMarkStack->isEmpty();
</span><span class="cx">
</span><span class="cx"> if (Options::logGC()) {
</span><del>- dataLog(m_collectorSlotVisitor->collectorMarkStack().size(), "+", m_mutatorMarkStack->size() + m_collectorSlotVisitor->mutatorMarkStack().size(), ", a=", m_bytesAllocatedThisCycle / 1024, " kb, b=", m_barriersExecuted, ", mu=", scheduler.currentDecision().targetMutatorUtilization(), " ");
</del><ins>+ dataLog(m_collectorSlotVisitor->collectorMarkStack().size(), "+", m_mutatorMarkStack->size() + m_collectorSlotVisitor->mutatorMarkStack().size(), ", a=", m_bytesAllocatedThisCycle / 1024, " kb, b=", m_barriersExecuted, ", mu=", scheduler.currentDecision().targetMutatorUtilization(), ", ws=", weakSetCount, ", cs=", m_constraints.size(), " ");
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // We want to do this to conservatively ensure that we rescan any code blocks that are
</span><span class="lines">@@ -2220,27 +2225,10 @@
</span><span class="cx"> func(*slotVisitor);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void Heap::writeBarrierOpaqueRootSlow(void* root)
-{
- ASSERT(mutatorShouldBeFenced());
-
- auto locker = holdLock(m_opaqueRootsMutex);
- m_opaqueRoots.add(root);
-}
-
-void Heap::addMutatorShouldBeFencedCache(bool& cache)
-{
- ASSERT(hasHeapAccess());
- cache = m_mutatorShouldBeFenced;
- m_mutatorShouldBeFencedCaches.append(&cache);
-}
-
</del><span class="cx"> void Heap::setMutatorShouldBeFenced(bool value)
</span><span class="cx"> {
</span><span class="cx"> m_mutatorShouldBeFenced = value;
</span><span class="cx"> m_barrierThreshold = value ? tautologicalThreshold : blackThreshold;
</span><del>- for (bool* cache : m_mutatorShouldBeFencedCaches)
- *cache = value;
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -127,8 +127,6 @@
</span><span class="cx"> WriteBarrierBuffer& writeBarrierBuffer() { return m_writeBarrierBuffer; }
</span><span class="cx"> void flushWriteBarrierBuffer(JSCell*);
</span><span class="cx">
</span><del>- void writeBarrierOpaqueRoot(void*);
-
</del><span class="cx"> Heap(VM*, HeapType);
</span><span class="cx"> ~Heap();
</span><span class="cx"> void lastChanceToFinalize();
</span><span class="lines">@@ -352,8 +350,6 @@
</span><span class="cx"> void preventCollection();
</span><span class="cx"> void allowCollection();
</span><span class="cx">
</span><del>- JS_EXPORT_PRIVATE void addMutatorShouldBeFencedCache(bool&);
-
</del><span class="cx"> #if USE(CF)
</span><span class="cx"> CFRunLoopRef runLoop() const { return m_runLoop.get(); }
</span><span class="cx"> JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
</span><span class="lines">@@ -497,8 +493,6 @@
</span><span class="cx">
</span><span class="cx"> void forEachCodeBlockImpl(const ScopedLambda<bool(CodeBlock*)>&);
</span><span class="cx">
</span><del>- JS_EXPORT_PRIVATE void writeBarrierOpaqueRootSlow(void*);
-
</del><span class="cx"> void setMutatorShouldBeFenced(bool value);
</span><span class="cx">
</span><span class="cx"> const HeapType m_heapType;
</span><span class="lines">@@ -559,7 +553,6 @@
</span><span class="cx"> WriteBarrierBuffer m_writeBarrierBuffer;
</span><span class="cx"> bool m_mutatorShouldBeFenced { Options::forceFencedBarrier() };
</span><span class="cx"> unsigned m_barrierThreshold { Options::forceFencedBarrier() ? tautologicalThreshold : blackThreshold };
</span><del>- Vector<bool*> m_mutatorShouldBeFencedCaches;
</del><span class="cx">
</span><span class="cx"> VM* m_vm;
</span><span class="cx"> double m_lastFullGCLength;
</span><span class="lines">@@ -603,7 +596,8 @@
</span><span class="cx"> bool m_parallelMarkersShouldExit { false };
</span><span class="cx">
</span><span class="cx"> Lock m_opaqueRootsMutex;
</span><del>- HashSet<void*> m_opaqueRoots;
</del><ins>+ HashSet<const void*> m_opaqueRoots;
+ HashSet<JSCell*> m_constraints;
</ins><span class="cx">
</span><span class="cx"> static const size_t s_blockFragmentLength = 32;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapInlines.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapInlines.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/HeapInlines.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -370,10 +370,4 @@
</span><span class="cx"> stopIfNecessarySlow();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline void Heap::writeBarrierOpaqueRoot(void* root)
-{
- if (mutatorShouldBeFenced())
- writeBarrierOpaqueRootSlow(root);
-}
-
</del><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkedSpacecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkedSpace.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkedSpace.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/MarkedSpace.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -359,10 +359,11 @@
</span><span class="cx"> m_allocatorForEmptyAllocation = m_firstAllocator;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void MarkedSpace::visitWeakSets(HeapRootVisitor& heapRootVisitor)
</del><ins>+size_t MarkedSpace::visitWeakSets(HeapRootVisitor& heapRootVisitor)
</ins><span class="cx"> {
</span><ins>+ size_t count = 0;
</ins><span class="cx"> auto visit = [&] (WeakSet* weakSet) {
</span><del>- weakSet->visit(heapRootVisitor);
</del><ins>+ count += weakSet->visit(heapRootVisitor);
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> m_newActiveWeakSets.forEach(visit);
</span><span class="lines">@@ -369,6 +370,7 @@
</span><span class="cx">
</span><span class="cx"> if (m_heap->collectionScope() == CollectionScope::Full)
</span><span class="cx"> m_activeWeakSets.forEach(visit);
</span><ins>+ return count;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MarkedSpace::reapWeakSets()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkedSpaceh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkedSpace.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkedSpace.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/MarkedSpace.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -129,7 +129,7 @@
</span><span class="cx">
</span><span class="cx"> void prepareForAllocation();
</span><span class="cx">
</span><del>- void visitWeakSets(HeapRootVisitor&);
</del><ins>+ size_t visitWeakSets(HeapRootVisitor&);
</ins><span class="cx"> void reapWeakSets();
</span><span class="cx">
</span><span class="cx"> MarkedBlockSet& blocks() { return m_blocks; }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -356,8 +356,8 @@
</span><span class="cx">
</span><span class="cx"> if (false) {
</span><span class="cx"> dataLog("Visiting ", RawPointer(cell));
</span><del>- if (m_isVisitingMutatorStack)
- dataLog(" (mutator)");
</del><ins>+ if (!m_isFirstVisit)
+ dataLog(" (subsequent)");
</ins><span class="cx"> dataLog("\n");
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -391,11 +391,17 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (UNLIKELY(m_heapSnapshotBuilder)) {
</span><del>- if (!m_isVisitingMutatorStack)
</del><ins>+ if (m_isFirstVisit)
</ins><span class="cx"> m_heapSnapshotBuilder->appendNode(const_cast<JSCell*>(cell));
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void SlotVisitor::visitSubsequently(JSCell* cell)
+{
+ m_isFirstVisit = false;
+ visitChildren(cell);
+}
+
</ins><span class="cx"> void SlotVisitor::donateKnownParallel(MarkStackArray& from, MarkStackArray& to)
</span><span class="cx"> {
</span><span class="cx"> // NOTE: Because we re-try often, we can afford to be conservative, and
</span><span class="lines">@@ -465,7 +471,7 @@
</span><span class="cx"> updateMutatorIsStopped(locker);
</span><span class="cx"> if (!m_collectorStack.isEmpty()) {
</span><span class="cx"> m_collectorStack.refill();
</span><del>- m_isVisitingMutatorStack = false;
</del><ins>+ m_isFirstVisit = true;
</ins><span class="cx"> for (unsigned countdown = Options::minimumNumberOfScansBetweenRebalance(); m_collectorStack.canRemoveLast() && countdown--;)
</span><span class="cx"> visitChildren(m_collectorStack.removeLast());
</span><span class="cx"> } else if (!m_mutatorStack.isEmpty()) {
</span><span class="lines">@@ -473,7 +479,7 @@
</span><span class="cx"> // We know for sure that we are visiting objects because of the barrier, not because of
</span><span class="cx"> // marking. Marking will visit an object exactly once. The barrier will visit it
</span><span class="cx"> // possibly many times, and always after it was already marked.
</span><del>- m_isVisitingMutatorStack = true;
</del><ins>+ m_isFirstVisit = false;
</ins><span class="cx"> for (unsigned countdown = Options::minimumNumberOfScansBetweenRebalance(); m_mutatorStack.canRemoveLast() && countdown--;)
</span><span class="cx"> visitChildren(m_mutatorStack.removeLast());
</span><span class="cx"> }
</span><span class="lines">@@ -481,7 +487,7 @@
</span><span class="cx"> donateKnownParallel();
</span><span class="cx"> }
</span><span class="cx">
</span><del>- mergeOpaqueRootsIfNecessary();
</del><ins>+ mergeIfNecessary();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool SlotVisitor::didReachTermination()
</span><span class="lines">@@ -597,6 +603,7 @@
</span><span class="cx"> if (Options::numberOfGCMarkers() == 1) {
</span><span class="cx"> // Put directly into the shared HashSet.
</span><span class="cx"> m_heap.m_opaqueRoots.add(root);
</span><ins>+ m_heap.m_constraints.add(m_currentCell);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx"> // Put into the local set, but merge with the shared one every once in
</span><span class="lines">@@ -603,8 +610,19 @@
</span><span class="cx"> // a while to make sure that the local sets don't grow too large.
</span><span class="cx"> mergeOpaqueRootsIfProfitable();
</span><span class="cx"> m_opaqueRoots.add(root);
</span><ins>+ m_constraints.add(m_currentCell);
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+void SlotVisitor::rescanAsConstraint()
+{
+ if (Options::numberOfGCMarkers() == 1) {
+ m_heap.m_constraints.add(m_currentCell);
+ return;
+ }
+
+ m_constraints.add(m_currentCell);
+}
+
</ins><span class="cx"> bool SlotVisitor::containsOpaqueRoot(void* root) const
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!m_isInParallelMode);
</span><span class="lines">@@ -621,13 +639,13 @@
</span><span class="cx"> return MixedTriState;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void SlotVisitor::mergeOpaqueRootsIfNecessary()
</del><ins>+void SlotVisitor::mergeIfNecessary()
</ins><span class="cx"> {
</span><del>- if (m_opaqueRoots.isEmpty())
</del><ins>+ if (m_opaqueRoots.isEmpty() && m_constraints.isEmpty())
</ins><span class="cx"> return;
</span><del>- mergeOpaqueRoots();
</del><ins>+ mergeOpaqueRootsAndConstraints();
</ins><span class="cx"> }
</span><del>-
</del><ins>+
</ins><span class="cx"> void SlotVisitor::mergeOpaqueRootsIfProfitable()
</span><span class="cx"> {
</span><span class="cx"> if (static_cast<unsigned>(m_opaqueRoots.size()) < Options::opaqueRootMergeThreshold())
</span><span class="lines">@@ -660,6 +678,19 @@
</span><span class="cx"> m_opaqueRoots.clear();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void SlotVisitor::mergeOpaqueRootsAndConstraints()
+{
+ {
+ std::lock_guard<Lock> lock(m_heap.m_opaqueRootsMutex);
+ for (const void* root : m_opaqueRoots)
+ m_heap.m_opaqueRoots.add(root);
+ for (JSCell* constraint : m_constraints)
+ m_heap.m_constraints.add(constraint);
+ }
+ m_opaqueRoots.clear();
+ m_constraints.clear();
+}
+
</ins><span class="cx"> void SlotVisitor::addWeakReferenceHarvester(WeakReferenceHarvester* weakReferenceHarvester)
</span><span class="cx"> {
</span><span class="cx"> m_heap.m_weakReferenceHarvesters.addThreadSafe(weakReferenceHarvester);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitor.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitor.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitor.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -85,7 +85,18 @@
</span><span class="cx"> void appendUnbarrieredReadOnlyPointer(T*);
</span><span class="cx"> void appendUnbarrieredReadOnlyValue(JSValue);
</span><span class="cx">
</span><ins>+ void visitSubsequently(JSCell*);
+
+ // Does your visitChildren do logic that depends on non-JS-object state that can
+ // change during the course of a GC, or in between GCs? Then you should call this
+ // method! It will cause the GC to invoke your visitChildren method again just before
+ // terminating with the world stopped.
+ JS_EXPORT_PRIVATE void rescanAsConstraint();
+
+ // Implies rescanAsConstraint, so you don't have to call rescanAsConstraint() if you
+ // call this unconditionally.
</ins><span class="cx"> JS_EXPORT_PRIVATE void addOpaqueRoot(void*);
</span><ins>+
</ins><span class="cx"> JS_EXPORT_PRIVATE bool containsOpaqueRoot(void*) const;
</span><span class="cx"> TriState containsOpaqueRootTriState(void*) const;
</span><span class="cx">
</span><span class="lines">@@ -108,6 +119,8 @@
</span><span class="cx">
</span><span class="cx"> SharedDrainResult drainInParallel(MonotonicTime timeout = MonotonicTime::infinity());
</span><span class="cx"> SharedDrainResult drainInParallelPassively(MonotonicTime timeout = MonotonicTime::infinity());
</span><ins>+
+ void mergeIfNecessary();
</ins><span class="cx">
</span><span class="cx"> // This informs the GC about auxiliary of some size that we are keeping alive. If you don't do
</span><span class="cx"> // this then the space will be freed at end of GC.
</span><span class="lines">@@ -127,8 +140,6 @@
</span><span class="cx">
</span><span class="cx"> HeapVersion markingVersion() const { return m_markingVersion; }
</span><span class="cx">
</span><del>- void mergeOpaqueRootsIfNecessary();
-
</del><span class="cx"> bool mutatorIsStopped() const { return m_mutatorIsStopped; }
</span><span class="cx">
</span><span class="cx"> Lock& rightToRun() { return m_rightToRun; }
</span><span class="lines">@@ -167,7 +178,9 @@
</span><span class="cx">
</span><span class="cx"> void noteLiveAuxiliaryCell(HeapCell*);
</span><span class="cx">
</span><del>- JS_EXPORT_PRIVATE void mergeOpaqueRoots();
</del><ins>+ void mergeOpaqueRoots();
+ void mergeOpaqueRootsAndConstraints();
+
</ins><span class="cx"> void mergeOpaqueRootsIfProfitable();
</span><span class="cx">
</span><span class="cx"> void visitChildren(const JSCell*);
</span><span class="lines">@@ -181,6 +194,7 @@
</span><span class="cx"> MarkStackArray m_collectorStack;
</span><span class="cx"> MarkStackArray m_mutatorStack;
</span><span class="cx"> OpaqueRootSet m_opaqueRoots; // Handle-owning data structures not visible to the garbage collector.
</span><ins>+ HashSet<JSCell*> m_constraints;
</ins><span class="cx">
</span><span class="cx"> size_t m_bytesVisited;
</span><span class="cx"> size_t m_visitCount;
</span><span class="lines">@@ -192,7 +206,7 @@
</span><span class="cx">
</span><span class="cx"> HeapSnapshotBuilder* m_heapSnapshotBuilder { nullptr };
</span><span class="cx"> JSCell* m_currentCell { nullptr };
</span><del>- bool m_isVisitingMutatorStack { false };
</del><ins>+ bool m_isFirstVisit { false };
</ins><span class="cx"> bool m_mutatorIsStopped { false };
</span><span class="cx"> bool m_canOptimizeForStoppedMutator { false };
</span><span class="cx"> Lock m_rightToRun;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -95,7 +95,7 @@
</span><span class="cx">
</span><span class="cx"> inline void SlotVisitor::reportExtraMemoryVisited(size_t size)
</span><span class="cx"> {
</span><del>- if (!m_isVisitingMutatorStack)
</del><ins>+ if (m_isFirstVisit)
</ins><span class="cx"> heap()->reportExtraMemoryVisited(size);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -102,7 +102,7 @@
</span><span class="cx"> #if ENABLE(RESOURCE_USAGE)
</span><span class="cx"> inline void SlotVisitor::reportExternalMemoryVisited(size_t size)
</span><span class="cx"> {
</span><del>- if (!m_isVisitingMutatorStack)
</del><ins>+ if (m_isFirstVisit)
</ins><span class="cx"> heap()->reportExternalMemoryVisited(size);
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="lines">@@ -127,7 +127,7 @@
</span><span class="cx"> if (ASSERT_DISABLED)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (!m_isVisitingMutatorStack) {
</del><ins>+ if (m_isFirstVisit) {
</ins><span class="cx"> // This is the first visit so we don't need to remove anything.
</span><span class="cx"> return;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapWeakBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/WeakBlock.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/WeakBlock.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/WeakBlock.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -97,13 +97,14 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<typename ContainerType>
</span><del>-void WeakBlock::specializedVisit(ContainerType& container, HeapRootVisitor& heapRootVisitor)
</del><ins>+size_t WeakBlock::specializedVisit(ContainerType& container, HeapRootVisitor& heapRootVisitor)
</ins><span class="cx"> {
</span><span class="cx"> SlotVisitor& visitor = heapRootVisitor.visitor();
</span><span class="cx">
</span><span class="cx"> HeapVersion markingVersion = visitor.markingVersion();
</span><span class="cx">
</span><del>- for (size_t i = 0; i < weakImplCount(); ++i) {
</del><ins>+ size_t count = weakImplCount();
+ for (size_t i = 0; i < count; ++i) {
</ins><span class="cx"> WeakImpl* weakImpl = &weakImpls()[i];
</span><span class="cx"> if (weakImpl->state() != WeakImpl::Live)
</span><span class="cx"> continue;
</span><span class="lines">@@ -121,21 +122,22 @@
</span><span class="cx">
</span><span class="cx"> heapRootVisitor.visit(&const_cast<JSValue&>(jsValue));
</span><span class="cx"> }
</span><ins>+
+ return count;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void WeakBlock::visit(HeapRootVisitor& heapRootVisitor)
</del><ins>+size_t WeakBlock::visit(HeapRootVisitor& heapRootVisitor)
</ins><span class="cx"> {
</span><span class="cx"> // If a block is completely empty, a visit won't have any effect.
</span><span class="cx"> if (isEmpty())
</span><del>- return;
</del><ins>+ return 0;
</ins><span class="cx">
</span><span class="cx"> // If this WeakBlock doesn't belong to a CellContainer, we won't even be here.
</span><span class="cx"> ASSERT(m_container);
</span><span class="cx">
</span><span class="cx"> if (m_container.isLargeAllocation())
</span><del>- specializedVisit(m_container.largeAllocation(), heapRootVisitor);
- else
- specializedVisit(m_container.markedBlock(), heapRootVisitor);
</del><ins>+ return specializedVisit(m_container.largeAllocation(), heapRootVisitor);
+ return specializedVisit(m_container.markedBlock(), heapRootVisitor);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void WeakBlock::reap()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapWeakBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/WeakBlock.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/WeakBlock.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/WeakBlock.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -63,7 +63,7 @@
</span><span class="cx"> void sweep();
</span><span class="cx"> SweepResult takeSweepResult();
</span><span class="cx">
</span><del>- void visit(HeapRootVisitor&);
</del><ins>+ size_t visit(HeapRootVisitor&);
</ins><span class="cx"> void reap();
</span><span class="cx">
</span><span class="cx"> void lastChanceToFinalize();
</span><span class="lines">@@ -73,7 +73,7 @@
</span><span class="cx"> static FreeCell* asFreeCell(WeakImpl*);
</span><span class="cx">
</span><span class="cx"> template<typename ContainerType>
</span><del>- void specializedVisit(ContainerType&, HeapRootVisitor&);
</del><ins>+ size_t specializedVisit(ContainerType&, HeapRootVisitor&);
</ins><span class="cx">
</span><span class="cx"> explicit WeakBlock(CellContainer);
</span><span class="cx"> void finalize(WeakImpl*);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapWeakSeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/WeakSet.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/WeakSet.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/JavaScriptCore/heap/WeakSet.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -53,7 +53,7 @@
</span><span class="cx">
</span><span class="cx"> bool isEmpty() const;
</span><span class="cx">
</span><del>- unsigned visit(HeapRootVisitor&);
</del><ins>+ size_t visit(HeapRootVisitor&);
</ins><span class="cx"> void reap();
</span><span class="cx"> void sweep();
</span><span class="cx"> void shrink();
</span><span class="lines">@@ -106,13 +106,11 @@
</span><span class="cx"> block->lastChanceToFinalize();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline unsigned WeakSet::visit(HeapRootVisitor& visitor)
</del><ins>+inline size_t WeakSet::visit(HeapRootVisitor& visitor)
</ins><span class="cx"> {
</span><del>- unsigned count = 0;
- for (WeakBlock* block = m_blocks.head(); block; block = block->next()) {
- count++;
- block->visit(visitor);
- }
</del><ins>+ size_t count = 0;
+ for (WeakBlock* block = m_blocks.head(); block; block = block->next())
+ count += block->visit(visitor);
</ins><span class="cx"> return count;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/ChangeLog 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -1,3 +1,51 @@
</span><ins>+2016-12-13 Filip Pizlo <fpizlo@apple.com>
+
+ Make opaque root scanning truly constraint-based
+ https://bugs.webkit.org/show_bug.cgi?id=165760
+
+ Reviewed by Saam Barati.
+
+ No new tests yet. I think that writing tests for this is a big investigation:
+ https://bugs.webkit.org/show_bug.cgi?id=165808
+
+ Remove the previous advancing wavefront DOM write barrier. I don't think this will scale
+ very well. It's super confusing.
+
+ This change makes it so that visitChildren can become a GC constraint that executes as
+ part of the fixpoint. This changes all WebCore visitChildren methods that do opaque
+ roots into constraints.
+
+ * bindings/js/CommonVM.cpp:
+ (WebCore::commonVMSlow):
+ (WebCore::writeBarrierOpaqueRootSlow): Deleted.
+ * bindings/js/CommonVM.h:
+ (WebCore::writeBarrierOpaqueRoot): Deleted.
+ * bindings/js/JSAttrCustom.cpp:
+ (WebCore::JSAttr::visitAdditionalChildren):
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::visitAdditionalChildren):
+ * bindings/js/JSIDBCursorCustom.cpp:
+ (WebCore::JSIDBCursor::visitAdditionalChildren):
+ * bindings/js/JSMessageChannelCustom.cpp:
+ (WebCore::JSMessageChannel::visitAdditionalChildren):
+ * bindings/js/JSMessagePortCustom.cpp:
+ (WebCore::JSMessagePort::visitAdditionalChildren):
+ * bindings/js/JSNodeIteratorCustom.cpp:
+ (WebCore::JSNodeIterator::visitAdditionalChildren):
+ * bindings/js/JSTextTrackCueCustom.cpp:
+ (WebCore::JSTextTrackCue::visitAdditionalChildren):
+ * bindings/js/JSTreeWalkerCustom.cpp:
+ (WebCore::JSTreeWalker::visitAdditionalChildren):
+ * bindings/js/JSWorkerGlobalScopeCustom.cpp:
+ (WebCore::JSWorkerGlobalScope::visitAdditionalChildren):
+ * bindings/js/JSXMLHttpRequestCustom.cpp:
+ (WebCore::JSXMLHttpRequest::visitAdditionalChildren):
+ * bindings/js/JSXPathResultCustom.cpp:
+ (WebCore::JSXPathResult::visitAdditionalChildren):
+ * dom/ContainerNodeAlgorithms.cpp:
+ (WebCore::notifyChildNodeInserted):
+ (WebCore::notifyChildNodeRemoved):
+
</ins><span class="cx"> 2016-12-12 Sam Weinig <sam@webkit.org>
</span><span class="cx">
</span><span class="cx"> [WebIDL] Remove use of Dictionary in ApplePaySession
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsCommonVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/CommonVM.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/CommonVM.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/CommonVM.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -38,7 +38,6 @@
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> VM* g_commonVMOrNull;
</span><del>-bool g_opaqueRootWriteBarrierEnabled;
</del><span class="cx">
</span><span class="cx"> VM& commonVMSlow()
</span><span class="cx"> {
</span><span class="lines">@@ -56,7 +55,6 @@
</span><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> g_commonVMOrNull->setGlobalConstRedeclarationShouldThrow(Settings::globalConstRedeclarationShouldThrow());
</span><del>- g_commonVMOrNull->heap.addMutatorShouldBeFencedCache(g_opaqueRootWriteBarrierEnabled);
</del><span class="cx">
</span><span class="cx"> initNormalWorldClientData(g_commonVMOrNull);
</span><span class="cx">
</span><span class="lines">@@ -63,11 +61,5 @@
</span><span class="cx"> return *g_commonVMOrNull;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void writeBarrierOpaqueRootSlow(void* root)
-{
- if (VM* vm = g_commonVMOrNull)
- vm->heap.writeBarrierOpaqueRoot(root);
-}
-
</del><span class="cx"> } // namespace WebCore
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsCommonVMh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/CommonVM.h (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/CommonVM.h 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/CommonVM.h 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -32,10 +32,8 @@
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> WEBCORE_EXPORT extern JSC::VM* g_commonVMOrNull;
</span><del>-WEBCORE_EXPORT extern bool g_opaqueRootWriteBarrierEnabled;
</del><span class="cx">
</span><span class="cx"> WEBCORE_EXPORT JSC::VM& commonVMSlow();
</span><del>-WEBCORE_EXPORT void writeBarrierOpaqueRootSlow(void*);
</del><span class="cx">
</span><span class="cx"> inline JSC::VM& commonVM()
</span><span class="cx"> {
</span><span class="lines">@@ -44,12 +42,5 @@
</span><span class="cx"> return commonVMSlow();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-template<typename Func>
-void writeBarrierOpaqueRoot(const Func& rootThunk)
-{
- if (g_opaqueRootWriteBarrierEnabled)
- writeBarrierOpaqueRootSlow(rootThunk());
-}
-
</del><span class="cx"> } // namespace WebCore
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSAttrCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSAttrCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSAttrCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSAttrCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -35,6 +35,7 @@
</span><span class="cx">
</span><span class="cx"> void JSAttr::visitAdditionalChildren(JSC::SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
</ins><span class="cx"> if (Element* element = wrapped().ownerElement())
</span><span class="cx"> visitor.addOpaqueRoot(root(element));
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSDOMWindowCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -51,6 +51,7 @@
</span><span class="cx">
</span><span class="cx"> void JSDOMWindow::visitAdditionalChildren(SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
</ins><span class="cx"> if (Frame* frame = wrapped().frame())
</span><span class="cx"> visitor.addOpaqueRoot(frame);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSIDBCursorCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -39,6 +39,7 @@
</span><span class="cx">
</span><span class="cx"> void JSIDBCursor::visitAdditionalChildren(SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
</ins><span class="cx"> auto& cursor = wrapped();
</span><span class="cx"> if (auto* request = cursor.request())
</span><span class="cx"> visitor.addOpaqueRoot(request);
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSMessageChannelCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSMessageChannelCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSMessageChannelCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSMessageChannelCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -35,6 +35,8 @@
</span><span class="cx">
</span><span class="cx"> void JSMessageChannel::visitAdditionalChildren(JSC::SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> if (MessagePort* port = wrapped().port1())
</span><span class="cx"> visitor.addOpaqueRoot(port);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSMessagePortCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSMessagePortCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSMessagePortCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSMessagePortCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -33,6 +33,8 @@
</span><span class="cx">
</span><span class="cx"> void JSMessagePort::visitAdditionalChildren(SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> // If we have a locally entangled port, we can directly mark it as reachable. Ports that are remotely entangled are marked in-use by markActiveObjectsForContext().
</span><span class="cx"> if (MessagePort* port = wrapped().locallyEntangledPort())
</span><span class="cx"> visitor.addOpaqueRoot(port);
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSNodeIteratorCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSNodeIteratorCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSNodeIteratorCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSNodeIteratorCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -27,6 +27,8 @@
</span><span class="cx">
</span><span class="cx"> void JSNodeIterator::visitAdditionalChildren(JSC::SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> if (NodeFilter* filter = wrapped().filter())
</span><span class="cx"> visitor.addOpaqueRoot(filter);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSTextTrackCueCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -77,6 +77,8 @@
</span><span class="cx">
</span><span class="cx"> void JSTextTrackCue::visitAdditionalChildren(SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> if (TextTrack* textTrack = wrapped().track())
</span><span class="cx"> visitor.addOpaqueRoot(root(textTrack));
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSTreeWalkerCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSTreeWalkerCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSTreeWalkerCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSTreeWalkerCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -27,6 +27,8 @@
</span><span class="cx">
</span><span class="cx"> void JSTreeWalker::visitAdditionalChildren(JSC::SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> if (NodeFilter* filter = wrapped().filter())
</span><span class="cx"> visitor.addOpaqueRoot(filter);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSWorkerGlobalScopeCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -36,6 +36,8 @@
</span><span class="cx">
</span><span class="cx"> void JSWorkerGlobalScope::visitAdditionalChildren(SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> if (auto* location = wrapped().optionalLocation())
</span><span class="cx"> visitor.addOpaqueRoot(location);
</span><span class="cx"> if (auto* navigator = wrapped().optionalNavigator())
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSXMLHttpRequestCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -58,6 +58,8 @@
</span><span class="cx">
</span><span class="cx"> void JSXMLHttpRequest::visitAdditionalChildren(SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> if (XMLHttpRequestUpload* upload = wrapped().optionalUpload())
</span><span class="cx"> visitor.addOpaqueRoot(upload);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSXPathResultCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSXPathResultCustom.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSXPathResultCustom.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/bindings/js/JSXPathResultCustom.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -33,6 +33,8 @@
</span><span class="cx">
</span><span class="cx"> void JSXPathResult::visitAdditionalChildren(JSC::SlotVisitor& visitor)
</span><span class="cx"> {
</span><ins>+ visitor.rescanAsConstraint();
+
</ins><span class="cx"> auto& value = wrapped().value();
</span><span class="cx"> if (value.isNodeSet()) {
</span><span class="cx"> // FIXME: This looks like it might race, but I'm not sure.
</span></span></pre></div>
<a id="trunkSourceWebCoredomContainerNodeAlgorithmscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (209765 => 209766)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2016-12-13 19:44:06 UTC (rev 209765)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2016-12-13 19:54:15 UTC (rev 209766)
</span><span class="lines">@@ -26,7 +26,6 @@
</span><span class="cx"> #include "config.h"
</span><span class="cx"> #include "ContainerNodeAlgorithms.h"
</span><span class="cx">
</span><del>-#include "CommonVM.h"
</del><span class="cx"> #include "HTMLFrameOwnerElement.h"
</span><span class="cx"> #include "InspectorInstrumentation.h"
</span><span class="cx"> #include "NoEventDispatchAssertion.h"
</span><span class="lines">@@ -102,8 +101,6 @@
</span><span class="cx"> notifyNodeInsertedIntoDocument(insertionPoint, node, postInsertionNotificationTargets);
</span><span class="cx"> else if (is<ContainerNode>(node))
</span><span class="cx"> notifyNodeInsertedIntoTree(insertionPoint, downcast<ContainerNode>(node), postInsertionNotificationTargets);
</span><del>-
- writeBarrierOpaqueRoot([&insertionPoint] () -> void* { return insertionPoint.opaqueRoot(); });
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void notifyNodeRemovedFromDocument(ContainerNode& insertionPoint, Node& node)
</span><span class="lines">@@ -155,8 +152,6 @@
</span><span class="cx">
</span><span class="cx"> void notifyChildNodeRemoved(ContainerNode& insertionPoint, Node& child)
</span><span class="cx"> {
</span><del>- writeBarrierOpaqueRoot([&child] () -> void* { return &child; });
-
</del><span class="cx"> if (!child.inDocument()) {
</span><span class="cx"> if (is<ContainerNode>(child))
</span><span class="cx"> notifyNodeRemovedFromTree(insertionPoint, downcast<ContainerNode>(child));
</span></span></pre>
</div>
</div>
</body>
</html>