<!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>[205683] 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/205683">205683</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-09-08 18:22:05 -0700 (Thu, 08 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Heap::isMarked() shouldn't pay the price of concurrent lazy flipping
https://bugs.webkit.org/show_bug.cgi?id=161760
Reviewed by Mark Lam.
Source/JavaScriptCore:
To fix a race condition in marking, I made Heap::isMarked() and Heap::isLive() atomic by
using flipIfNecessaryConcurrently() instead of flipIfNecessary().
This introduces three unnecessary overheads:
- isLive() is not called by marking, so that change was not necessary.
- isMarked() gets calls many times outside of marking, so it shouldn't always do the
concurrent thing. This adds isMarkedConcurrently() for use in marking, and reverts
isMarked().
- isMarked() and isMarkedConcurrently() don't actually have to do the lazy flip. They can
return false if the flip is necessary.
I added a bunch of debug assertions to make sure that isLive() and isMarked() are not called
during marking.
If we needed to, we could remove most of the calls to isMarkedConcurrently(). As a kind of
optimization, CodeBlock does an initial fixpoint iteration during marking, and so all of the
code called from CodeBlock's fixpoint iterator needs to use isMarkedConcurrently(). But we
could probably arrange for CodeBlock only do fixpoint iterating during the weak reference
thing.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::visitWeakly):
(JSC::CodeBlock::shouldJettisonDueToOldAge):
(JSC::shouldMarkTransition):
(JSC::CodeBlock::propagateTransitions):
(JSC::CodeBlock::determineLiveness):
* bytecode/PolymorphicAccess.cpp:
(JSC::AccessCase::propagateTransitions):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::isLive):
(JSC::Heap::isMarked):
(JSC::Heap::isMarkedConcurrently):
* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::flipIfNecessarySlow):
(JSC::MarkedBlock::flipIfNecessaryConcurrentlySlow):
(JSC::MarkedBlock::needsFlip):
* heap/MarkedBlock.h:
(JSC::MarkedBlock::needsFlip):
(JSC::MarkedBlock::flipIfNecessary):
(JSC::MarkedBlock::flipIfNecessaryConcurrently):
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::appendToMarkStack):
(JSC::SlotVisitor::markAuxiliary):
(JSC::SlotVisitor::visitChildren):
* runtime/Structure.cpp:
(JSC::Structure::isCheapDuringGC):
(JSC::Structure::markIfCheap):
Source/WTF:
* wtf/MainThread.cpp:
(WTF::isMainThreadOrGCThread):
(WTF::mayBeGCThread):
* wtf/MainThread.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCodeBlockcpp">trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodePolymorphicAccesscpp">trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.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="#trunkSourceJavaScriptCoreheapHeapSnapshotBuildercpp">trunk/Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkedBlockcpp">trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkedBlockh">trunk/Source/JavaScriptCore/heap/MarkedBlock.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorcpp">trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructurecpp">trunk/Source/JavaScriptCore/runtime/Structure.cpp</a></li>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfMainThreadcpp">trunk/Source/WTF/wtf/MainThread.cpp</a></li>
<li><a href="#trunkSourceWTFwtfMainThreadh">trunk/Source/WTF/wtf/MainThread.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -1,3 +1,62 @@
</span><ins>+2016-09-08 Filip Pizlo <fpizlo@apple.com>
+
+ Heap::isMarked() shouldn't pay the price of concurrent lazy flipping
+ https://bugs.webkit.org/show_bug.cgi?id=161760
+
+ Reviewed by Mark Lam.
+
+ To fix a race condition in marking, I made Heap::isMarked() and Heap::isLive() atomic by
+ using flipIfNecessaryConcurrently() instead of flipIfNecessary().
+
+ This introduces three unnecessary overheads:
+
+ - isLive() is not called by marking, so that change was not necessary.
+
+ - isMarked() gets calls many times outside of marking, so it shouldn't always do the
+ concurrent thing. This adds isMarkedConcurrently() for use in marking, and reverts
+ isMarked().
+
+ - isMarked() and isMarkedConcurrently() don't actually have to do the lazy flip. They can
+ return false if the flip is necessary.
+
+ I added a bunch of debug assertions to make sure that isLive() and isMarked() are not called
+ during marking.
+
+ If we needed to, we could remove most of the calls to isMarkedConcurrently(). As a kind of
+ optimization, CodeBlock does an initial fixpoint iteration during marking, and so all of the
+ code called from CodeBlock's fixpoint iterator needs to use isMarkedConcurrently(). But we
+ could probably arrange for CodeBlock only do fixpoint iterating during the weak reference
+ thing.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::visitWeakly):
+ (JSC::CodeBlock::shouldJettisonDueToOldAge):
+ (JSC::shouldMarkTransition):
+ (JSC::CodeBlock::propagateTransitions):
+ (JSC::CodeBlock::determineLiveness):
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::AccessCase::propagateTransitions):
+ * heap/Heap.h:
+ * heap/HeapInlines.h:
+ (JSC::Heap::isLive):
+ (JSC::Heap::isMarked):
+ (JSC::Heap::isMarkedConcurrently):
+ * heap/MarkedBlock.cpp:
+ (JSC::MarkedBlock::flipIfNecessarySlow):
+ (JSC::MarkedBlock::flipIfNecessaryConcurrentlySlow):
+ (JSC::MarkedBlock::needsFlip):
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::needsFlip):
+ (JSC::MarkedBlock::flipIfNecessary):
+ (JSC::MarkedBlock::flipIfNecessaryConcurrently):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::appendToMarkStack):
+ (JSC::SlotVisitor::markAuxiliary):
+ (JSC::SlotVisitor::visitChildren):
+ * runtime/Structure.cpp:
+ (JSC::Structure::isCheapDuringGC):
+ (JSC::Structure::markIfCheap):
+
</ins><span class="cx"> 2016-09-08 Saam Barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> We should inline operationConvertJSValueToBoolean into JIT code
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCodeBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -2485,7 +2485,7 @@
</span><span class="cx"> if (!setByMe)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (Heap::isMarked(this))
</del><ins>+ if (Heap::isMarkedConcurrently(this))
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> if (shouldVisitStrongly()) {
</span><span class="lines">@@ -2628,7 +2628,7 @@
</span><span class="cx">
</span><span class="cx"> bool CodeBlock::shouldJettisonDueToOldAge()
</span><span class="cx"> {
</span><del>- if (Heap::isMarked(this))
</del><ins>+ if (Heap::isMarkedConcurrently(this))
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> if (UNLIKELY(Options::forceCodeBlockToJettisonDueToOldAge()))
</span><span class="lines">@@ -2643,10 +2643,10 @@
</span><span class="cx"> #if ENABLE(DFG_JIT)
</span><span class="cx"> static bool shouldMarkTransition(DFG::WeakReferenceTransition& transition)
</span><span class="cx"> {
</span><del>- if (transition.m_codeOrigin && !Heap::isMarked(transition.m_codeOrigin.get()))
</del><ins>+ if (transition.m_codeOrigin && !Heap::isMarkedConcurrently(transition.m_codeOrigin.get()))
</ins><span class="cx"> return false;
</span><span class="cx">
</span><del>- if (!Heap::isMarked(transition.m_from.get()))
</del><ins>+ if (!Heap::isMarkedConcurrently(transition.m_from.get()))
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> return true;
</span><span class="lines">@@ -2677,7 +2677,7 @@
</span><span class="cx"> m_vm->heap.structureIDTable().get(oldStructureID);
</span><span class="cx"> Structure* newStructure =
</span><span class="cx"> m_vm->heap.structureIDTable().get(newStructureID);
</span><del>- if (Heap::isMarked(oldStructure))
</del><ins>+ if (Heap::isMarkedConcurrently(oldStructure))
</ins><span class="cx"> visitor.appendUnbarrieredReadOnlyPointer(newStructure);
</span><span class="cx"> else
</span><span class="cx"> allAreMarkedSoFar = false;
</span><span class="lines">@@ -2749,7 +2749,7 @@
</span><span class="cx"> // GC we still have not proved liveness, then this code block is toast.
</span><span class="cx"> bool allAreLiveSoFar = true;
</span><span class="cx"> for (unsigned i = 0; i < dfgCommon->weakReferences.size(); ++i) {
</span><del>- if (!Heap::isMarked(dfgCommon->weakReferences[i].get())) {
</del><ins>+ if (!Heap::isMarkedConcurrently(dfgCommon->weakReferences[i].get())) {
</ins><span class="cx"> allAreLiveSoFar = false;
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="lines">@@ -2756,7 +2756,7 @@
</span><span class="cx"> }
</span><span class="cx"> if (allAreLiveSoFar) {
</span><span class="cx"> for (unsigned i = 0; i < dfgCommon->weakStructureReferences.size(); ++i) {
</span><del>- if (!Heap::isMarked(dfgCommon->weakStructureReferences[i].get())) {
</del><ins>+ if (!Heap::isMarkedConcurrently(dfgCommon->weakStructureReferences[i].get())) {
</ins><span class="cx"> allAreLiveSoFar = false;
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodePolymorphicAccesscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -555,7 +555,7 @@
</span><span class="cx">
</span><span class="cx"> switch (m_type) {
</span><span class="cx"> case Transition:
</span><del>- if (Heap::isMarked(m_structure->previousID()))
</del><ins>+ if (Heap::isMarkedConcurrently(m_structure->previousID()))
</ins><span class="cx"> visitor.appendUnbarrieredReadOnlyPointer(m_structure.get());
</span><span class="cx"> else
</span><span class="cx"> result = false;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -99,6 +99,7 @@
</span><span class="cx">
</span><span class="cx"> static bool isLive(const void*);
</span><span class="cx"> static bool isMarked(const void*);
</span><ins>+ static bool isMarkedConcurrently(const void*);
</ins><span class="cx"> static bool testAndSetMarked(HeapVersion, const void*);
</span><span class="cx"> static void setMarked(const void*);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapInlines.h (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapInlines.h 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/heap/HeapInlines.h 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -34,6 +34,7 @@
</span><span class="cx"> #include "Structure.h"
</span><span class="cx"> #include <type_traits>
</span><span class="cx"> #include <wtf/Assertions.h>
</span><ins>+#include <wtf/MainThread.h>
</ins><span class="cx"> #include <wtf/RandomNumber.h>
</span><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="lines">@@ -75,24 +76,39 @@
</span><span class="cx">
</span><span class="cx"> inline bool Heap::isLive(const void* rawCell)
</span><span class="cx"> {
</span><ins>+ ASSERT(!mayBeGCThread());
</ins><span class="cx"> HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
</span><span class="cx"> if (cell->isLargeAllocation())
</span><span class="cx"> return cell->largeAllocation().isLive();
</span><span class="cx"> MarkedBlock& block = cell->markedBlock();
</span><del>- block.flipIfNecessaryConcurrently(block.vm()->heap.objectSpace().version());
</del><ins>+ block.flipIfNecessary(block.vm()->heap.objectSpace().version());
</ins><span class="cx"> return block.handle().isLiveCell(cell);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ALWAYS_INLINE bool Heap::isMarked(const void* rawCell)
</span><span class="cx"> {
</span><ins>+ ASSERT(!mayBeGCThread());
</ins><span class="cx"> HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
</span><span class="cx"> if (cell->isLargeAllocation())
</span><span class="cx"> return cell->largeAllocation().isMarked();
</span><span class="cx"> MarkedBlock& block = cell->markedBlock();
</span><del>- block.flipIfNecessaryConcurrently(block.vm()->heap.objectSpace().version());
</del><ins>+ if (block.needsFlip(block.vm()->heap.objectSpace().version()))
+ return false;
</ins><span class="cx"> return block.isMarked(cell);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ALWAYS_INLINE bool Heap::isMarkedConcurrently(const void* rawCell)
+{
+ HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
+ if (cell->isLargeAllocation())
+ return cell->largeAllocation().isMarked();
+ MarkedBlock& block = cell->markedBlock();
+ if (block.needsFlip(block.vm()->heap.objectSpace().version()))
+ return false;
+ WTF::loadLoadFence();
+ return block.isMarked(cell);
+}
+
</ins><span class="cx"> ALWAYS_INLINE bool Heap::testAndSetMarked(HeapVersion version, const void* rawCell)
</span><span class="cx"> {
</span><span class="cx"> HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapSnapshotBuildercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -66,7 +66,7 @@
</span><span class="cx"> void HeapSnapshotBuilder::appendNode(JSCell* cell)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_profiler.activeSnapshotBuilder() == this);
</span><del>- ASSERT(Heap::isMarked(cell));
</del><ins>+ ASSERT(Heap::isMarkedConcurrently(cell));
</ins><span class="cx">
</span><span class="cx"> if (hasExistingNodeForCell(cell))
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkedBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -357,7 +357,7 @@
</span><span class="cx">
</span><span class="cx"> void MarkedBlock::flipIfNecessarySlow()
</span><span class="cx"> {
</span><del>- ASSERT(m_version != vm()->heap.objectSpace().version());
</del><ins>+ ASSERT(needsFlip());
</ins><span class="cx"> clearMarks();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -364,7 +364,7 @@
</span><span class="cx"> void MarkedBlock::flipIfNecessaryConcurrentlySlow()
</span><span class="cx"> {
</span><span class="cx"> LockHolder locker(m_lock);
</span><del>- if (m_version != vm()->heap.objectSpace().version())
</del><ins>+ if (needsFlip())
</ins><span class="cx"> clearMarks();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -388,7 +388,7 @@
</span><span class="cx">
</span><span class="cx"> bool MarkedBlock::needsFlip()
</span><span class="cx"> {
</span><del>- return vm()->heap.objectSpace().version() != m_version;
</del><ins>+ return needsFlip(vm()->heap.objectSpace().version());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool MarkedBlock::Handle::needsFlip()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkedBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkedBlock.h (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkedBlock.h 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/heap/MarkedBlock.h 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -264,6 +264,7 @@
</span><span class="cx">
</span><span class="cx"> WeakSet& weakSet();
</span><span class="cx">
</span><ins>+ bool needsFlip(HeapVersion);
</ins><span class="cx"> bool needsFlip();
</span><span class="cx">
</span><span class="cx"> void flipIfNecessaryConcurrently(HeapVersion);
</span><span class="lines">@@ -462,15 +463,20 @@
</span><span class="cx"> return (reinterpret_cast<Bits>(p) - reinterpret_cast<Bits>(this)) / atomSize;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+inline bool MarkedBlock::needsFlip(HeapVersion heapVersion)
+{
+ return heapVersion != m_version;
+}
+
</ins><span class="cx"> inline void MarkedBlock::flipIfNecessary(HeapVersion heapVersion)
</span><span class="cx"> {
</span><del>- if (UNLIKELY(heapVersion != m_version))
</del><ins>+ if (UNLIKELY(needsFlip(heapVersion)))
</ins><span class="cx"> flipIfNecessarySlow();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline void MarkedBlock::flipIfNecessaryConcurrently(HeapVersion heapVersion)
</span><span class="cx"> {
</span><del>- if (UNLIKELY(heapVersion != m_version))
</del><ins>+ if (UNLIKELY(needsFlip(heapVersion)))
</ins><span class="cx"> flipIfNecessaryConcurrentlySlow();
</span><span class="cx"> WTF::loadLoadFence();
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -224,7 +224,7 @@
</span><span class="cx"> template<typename ContainerType>
</span><span class="cx"> ALWAYS_INLINE void SlotVisitor::appendToMarkStack(ContainerType& container, JSCell* cell)
</span><span class="cx"> {
</span><del>- ASSERT(Heap::isMarked(cell));
</del><ins>+ ASSERT(Heap::isMarkedConcurrently(cell));
</ins><span class="cx"> ASSERT(!cell->isZapped());
</span><span class="cx">
</span><span class="cx"> container.noteMarked();
</span><span class="lines">@@ -247,7 +247,7 @@
</span><span class="cx"> ASSERT(cell->heap() == heap());
</span><span class="cx">
</span><span class="cx"> if (Heap::testAndSetMarked(m_version, cell)) {
</span><del>- RELEASE_ASSERT(Heap::isMarked(cell));
</del><ins>+ RELEASE_ASSERT(Heap::isMarkedConcurrently(cell));
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -292,7 +292,7 @@
</span><span class="cx">
</span><span class="cx"> ALWAYS_INLINE void SlotVisitor::visitChildren(const JSCell* cell)
</span><span class="cx"> {
</span><del>- ASSERT(Heap::isMarked(cell));
</del><ins>+ ASSERT(Heap::isMarkedConcurrently(cell));
</ins><span class="cx">
</span><span class="cx"> SetCurrentCellScope currentCellScope(*this, cell);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructurecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Structure.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Structure.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/JavaScriptCore/runtime/Structure.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -1140,14 +1140,14 @@
</span><span class="cx"> // has any large property names.
</span><span class="cx"> // https://bugs.webkit.org/show_bug.cgi?id=157334
</span><span class="cx">
</span><del>- return (!m_globalObject || Heap::isMarked(m_globalObject.get()))
- && (!storedPrototypeObject() || Heap::isMarked(storedPrototypeObject()));
</del><ins>+ return (!m_globalObject || Heap::isMarkedConcurrently(m_globalObject.get()))
+ && (!storedPrototypeObject() || Heap::isMarkedConcurrently(storedPrototypeObject()));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool Structure::markIfCheap(SlotVisitor& visitor)
</span><span class="cx"> {
</span><span class="cx"> if (!isCheapDuringGC())
</span><del>- return Heap::isMarked(this);
</del><ins>+ return Heap::isMarkedConcurrently(this);
</ins><span class="cx">
</span><span class="cx"> visitor.appendUnbarrieredReadOnlyPointer(this);
</span><span class="cx"> return true;
</span></span></pre></div>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/ChangeLog 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/WTF/ChangeLog 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2016-09-08 Filip Pizlo <fpizlo@apple.com>
+
+ Heap::isMarked() shouldn't pay the price of concurrent lazy flipping
+ https://bugs.webkit.org/show_bug.cgi?id=161760
+
+ Reviewed by Mark Lam.
+
+ * wtf/MainThread.cpp:
+ (WTF::isMainThreadOrGCThread):
+ (WTF::mayBeGCThread):
+ * wtf/MainThread.h:
+
</ins><span class="cx"> 2016-09-08 Myles C. Maxfield <mmaxfield@apple.com>
</span><span class="cx">
</span><span class="cx"> Support new emoji group candidates
</span></span></pre></div>
<a id="trunkSourceWTFwtfMainThreadcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/MainThread.cpp (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/MainThread.cpp 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/WTF/wtf/MainThread.cpp 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2007, 2008, 2015 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2007, 2008, 2015-2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -210,10 +210,15 @@
</span><span class="cx">
</span><span class="cx"> bool isMainThreadOrGCThread()
</span><span class="cx"> {
</span><del>- if (isGCThread->isSet() && **isGCThread)
</del><ins>+ if (mayBeGCThread())
</ins><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> return isMainThread();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool mayBeGCThread()
+{
+ return isGCThread->isSet() && **isGCThread;
+}
+
</ins><span class="cx"> } // namespace WTF
</span></span></pre></div>
<a id="trunkSourceWTFwtfMainThreadh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/MainThread.h (205682 => 205683)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/MainThread.h 2016-09-09 01:06:47 UTC (rev 205682)
+++ trunk/Source/WTF/wtf/MainThread.h 2016-09-09 01:22:05 UTC (rev 205683)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2007, 2008, 2010 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2007, 2008, 2010, 2016 Apple Inc. All rights reserved.
</ins><span class="cx"> * Copyright (C) 2007 Justin Haygood (jhaygood@reaktix.com)
</span><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="lines">@@ -68,6 +68,7 @@
</span><span class="cx"> void initializeGCThreads();
</span><span class="cx">
</span><span class="cx"> WTF_EXPORT_PRIVATE void registerGCThread();
</span><ins>+WTF_EXPORT_PRIVATE bool mayBeGCThread();
</ins><span class="cx"> WTF_EXPORT_PRIVATE bool isMainThreadOrGCThread();
</span><span class="cx">
</span><span class="cx"> // NOTE: these functions are internal to the callOnMainThread implementation.
</span><span class="lines">@@ -88,15 +89,16 @@
</span><span class="cx"> } // namespace WTF
</span><span class="cx">
</span><span class="cx"> using WTF::callOnMainThread;
</span><del>-#if PLATFORM(COCOA)
-using WTF::callOnWebThreadOrDispatchAsyncOnMainThread;
-#endif
-using WTF::setMainThreadCallbacksPaused;
</del><ins>+using WTF::canAccessThreadLocalDataForThread;
</ins><span class="cx"> using WTF::isMainThread;
</span><span class="cx"> using WTF::isMainThreadOrGCThread;
</span><del>-using WTF::canAccessThreadLocalDataForThread;
</del><span class="cx"> using WTF::isUIThread;
</span><span class="cx"> using WTF::isWebThread;
</span><ins>+using WTF::mayBeGCThread;
+using WTF::setMainThreadCallbacksPaused;
+#if PLATFORM(COCOA)
+using WTF::callOnWebThreadOrDispatchAsyncOnMainThread;
+#endif
</ins><span class="cx"> #if USE(WEB_THREAD)
</span><span class="cx"> using WTF::initializeWebThread;
</span><span class="cx"> using WTF::initializeApplicationUIThreadIdentifier;
</span></span></pre>
</div>
</div>
</body>
</html>