<!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>[209954] trunk/Source/JavaScriptCore</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/209954">209954</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-12-16 18:41:05 -0800 (Fri, 16 Dec 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>CellState should have members with accurate names
https://bugs.webkit.org/show_bug.cgi?id=165969

Reviewed by Mark Lam.
        
This once again renames the members in CellState. I wanted to convey the following
pieces of information in the names:
        
- What does the state mean for Generational GC?
- What does the state mean for Concurrent GC?
- Does the state guarantee what it means, or is there some contingency?
        
The names I came up with are:
        
PossiblyOldOrBlack: An object in this state may be old, or may be black, depending on
    other things. If the mark bit is set then the object is either black or being
    blackened as we speak. It's going to survive the GC, so it will be old, but may be
    new now. In between GCs, objects in this state are definitely old. If the mark bit
    is not set, then the object is actually old and white.
        
DefinitelyNewAndWhite: The object was just allocated so it is white (not marked) and
    new.
        
DefinitelyGrey: The object is definitely grey - it will be rescanned in the future. It
    may be new or old depending on other things.

* heap/CellState.h:
* heap/Heap.cpp:
(JSC::Heap::addToRememberedSet):
(JSC::Heap::writeBarrierSlowPath):
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::appendJSCellOrAuxiliary):
(JSC::SlotVisitor::setMarkedAndAppendToMarkStack):
(JSC::SlotVisitor::appendToMarkStack):
(JSC::SlotVisitor::visitChildren):
* runtime/JSCellInlines.h:
(JSC::JSCell::JSCell):
* runtime/StructureIDBlob.h:
(JSC::StructureIDBlob::StructureIDBlob):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapCellStateh">trunk/Source/JavaScriptCore/heap/CellState.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorcpp">trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSCellInlinesh">trunk/Source/JavaScriptCore/runtime/JSCellInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructureIDBlobh">trunk/Source/JavaScriptCore/runtime/StructureIDBlob.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (209953 => 209954)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-12-17 02:41:05 UTC (rev 209954)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2016-12-16  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
+        CellState should have members with accurate names
+        https://bugs.webkit.org/show_bug.cgi?id=165969
+
+        Reviewed by Mark Lam.
+        
+        This once again renames the members in CellState. I wanted to convey the following
+        pieces of information in the names:
+        
+        - What does the state mean for Generational GC?
+        - What does the state mean for Concurrent GC?
+        - Does the state guarantee what it means, or is there some contingency?
+        
+        The names I came up with are:
+        
+        PossiblyOldOrBlack: An object in this state may be old, or may be black, depending on
+            other things. If the mark bit is set then the object is either black or being
+            blackened as we speak. It's going to survive the GC, so it will be old, but may be
+            new now. In between GCs, objects in this state are definitely old. If the mark bit
+            is not set, then the object is actually old and white.
+        
+        DefinitelyNewAndWhite: The object was just allocated so it is white (not marked) and
+            new.
+        
+        DefinitelyGrey: The object is definitely grey - it will be rescanned in the future. It
+            may be new or old depending on other things.
+
+        * heap/CellState.h:
+        * heap/Heap.cpp:
+        (JSC::Heap::addToRememberedSet):
+        (JSC::Heap::writeBarrierSlowPath):
+        * heap/SlotVisitor.cpp:
+        (JSC::SlotVisitor::appendJSCellOrAuxiliary):
+        (JSC::SlotVisitor::setMarkedAndAppendToMarkStack):
+        (JSC::SlotVisitor::appendToMarkStack):
+        (JSC::SlotVisitor::visitChildren):
+        * runtime/JSCellInlines.h:
+        (JSC::JSCell::JSCell):
+        * runtime/StructureIDBlob.h:
+        (JSC::StructureIDBlob::StructureIDBlob):
+
</ins><span class="cx"> 2016-12-16  Saam Barati  &lt;sbarati@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         B3::DoubleToFloatReduction will accidentally convince itself it converted a Phi from Double to Float and then convert uses of that Phi into a use of FloatToDouble(@Phi)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapCellStateh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/CellState.h (209953 => 209954)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/CellState.h        2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/JavaScriptCore/heap/CellState.h        2016-12-17 02:41:05 UTC (rev 209954)
</span><span class="lines">@@ -30,21 +30,19 @@
</span><span class="cx"> namespace JSC {
</span><span class="cx"> 
</span><span class="cx"> enum class CellState : uint8_t {
</span><del>-    // The object is either currently being scanned (anthracite) or it has finished being scanned
-    // (black). It could be scanned for the first time this GC, or the Nth time - if it's anthracite
-    // then the SlotVisitor knows. We explicitly say &quot;anthracite or black&quot; to emphasize the fact that
-    // this is no guarantee that we have finished scanning the object, unless you also know that all
-    // SlotVisitors are done.
-    AnthraciteOrBlack = 0,
</del><ins>+    // The object is either currently being scanned, or it has finished being scanned, or this
+    // is a full collection and it's actually a white object (you'd know because its mark bit
+    // would be clear).
+    PossiblyOldOrBlack = 0,
</ins><span class="cx">     
</span><span class="cx">     // The object is in eden. During GC, this means that the object has not been marked yet.
</span><del>-    NewWhite = 1,
</del><ins>+    DefinitelyNewAndWhite = 1,
</ins><span class="cx"> 
</span><span class="cx">     // The object is grey - i.e. it will be scanned.
</span><del>-    Grey = 2,
</del><ins>+    DefinitelyGrey = 2,
</ins><span class="cx"> };
</span><span class="cx"> 
</span><del>-static const unsigned blackThreshold = 0; // x &lt;= blackThreshold means x is AnthraciteOrBlack.
</del><ins>+static const unsigned blackThreshold = 0; // x &lt;= blackThreshold means x is PossiblyOldOrBlack.
</ins><span class="cx"> static const unsigned tautologicalThreshold = 100; // x &lt;= tautologicalThreshold is always true.
</span><span class="cx"> 
</span><span class="cx"> inline bool isWithinThreshold(CellState cellState, unsigned threshold)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (209953 => 209954)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp        2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp        2016-12-17 02:41:05 UTC (rev 209954)
</span><span class="lines">@@ -1011,11 +1011,11 @@
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="cx">     // It could be that the object was *just* marked. This means that the collector may set the
</span><del>-    // state to Grey and then to AnthraciteOrBlack at any time. It's OK for us to race with the
-    // collector here. If we win then this is accurate because the object _will_ get scanned again.
-    // If we lose then someone else will barrier the object again. That would be unfortunate but not
-    // the end of the world.
-    cell-&gt;setCellState(CellState::Grey);
</del><ins>+    // state to DefinitelyGrey and then to PossiblyOldOrBlack at any time. It's OK for us to
+    // race with the collector here. If we win then this is accurate because the object _will_
+    // get scanned again. If we lose then someone else will barrier the object again. That would
+    // be unfortunate but not the end of the world.
+    cell-&gt;setCellState(CellState::DefinitelyGrey);
</ins><span class="cx">     m_mutatorMarkStack-&gt;append(cell);
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -2016,7 +2016,7 @@
</span><span class="cx">         // In this case, the barrierThreshold is the tautological threshold, so from could still be
</span><span class="cx">         // not black. But we can't know for sure until we fire off a fence.
</span><span class="cx">         WTF::storeLoadFence();
</span><del>-        if (from-&gt;cellState() != CellState::AnthraciteOrBlack)
</del><ins>+        if (from-&gt;cellState() != CellState::PossiblyOldOrBlack)
</ins><span class="cx">             return;
</span><span class="cx">     }
</span><span class="cx">     
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp (209953 => 209954)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp        2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp        2016-12-17 02:41:05 UTC (rev 209954)
</span><span class="lines">@@ -204,7 +204,7 @@
</span><span class="cx">         JSCell* jsCell = static_cast&lt;JSCell*&gt;(heapCell);
</span><span class="cx">         validateCell(jsCell);
</span><span class="cx">         
</span><del>-        jsCell-&gt;setCellState(CellState::Grey);
</del><ins>+        jsCell-&gt;setCellState(CellState::DefinitelyGrey);
</ins><span class="cx"> 
</span><span class="cx">         appendToMarkStack(jsCell);
</span><span class="cx">         return;
</span><span class="lines">@@ -266,7 +266,7 @@
</span><span class="cx">     // Indicate that the object is grey and that:
</span><span class="cx">     // In case of concurrent GC: it's the first time it is grey in this GC cycle.
</span><span class="cx">     // In case of eden collection: it's a new object that became grey rather than an old remembered object.
</span><del>-    cell-&gt;setCellState(CellState::Grey);
</del><ins>+    cell-&gt;setCellState(CellState::DefinitelyGrey);
</ins><span class="cx">     
</span><span class="cx">     appendToMarkStack(container, cell);
</span><span class="cx"> }
</span><span class="lines">@@ -284,7 +284,7 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(Heap::isMarkedConcurrently(cell));
</span><span class="cx">     ASSERT(!cell-&gt;isZapped());
</span><del>-    ASSERT(cell-&gt;cellState() == CellState::Grey);
</del><ins>+    ASSERT(cell-&gt;cellState() == CellState::DefinitelyGrey);
</ins><span class="cx">     
</span><span class="cx">     container.noteMarked();
</span><span class="cx">     
</span><span class="lines">@@ -366,7 +366,7 @@
</span><span class="cx">     // not clear to me that it would be correct or profitable to bail here if the object is already
</span><span class="cx">     // black.
</span><span class="cx">     
</span><del>-    cell-&gt;setCellState(CellState::AnthraciteOrBlack);
</del><ins>+    cell-&gt;setCellState(CellState::PossiblyOldOrBlack);
</ins><span class="cx">     
</span><span class="cx">     WTF::storeLoadFence();
</span><span class="cx">     
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSCellInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSCellInlines.h (209953 => 209954)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSCellInlines.h        2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/JavaScriptCore/runtime/JSCellInlines.h        2016-12-17 02:41:05 UTC (rev 209954)
</span><span class="lines">@@ -41,7 +41,7 @@
</span><span class="cx"> namespace JSC {
</span><span class="cx"> 
</span><span class="cx"> inline JSCell::JSCell(CreatingEarlyCellTag)
</span><del>-    : m_cellState(CellState::NewWhite)
</del><ins>+    : m_cellState(CellState::DefinitelyNewAndWhite)
</ins><span class="cx"> {
</span><span class="cx">     ASSERT(!isCompilationThread());
</span><span class="cx"> }
</span><span class="lines">@@ -51,7 +51,7 @@
</span><span class="cx">     , m_indexingTypeAndMisc(structure-&gt;indexingTypeIncludingHistory())
</span><span class="cx">     , m_type(structure-&gt;typeInfo().type())
</span><span class="cx">     , m_flags(structure-&gt;typeInfo().inlineTypeFlags())
</span><del>-    , m_cellState(CellState::NewWhite)
</del><ins>+    , m_cellState(CellState::DefinitelyNewAndWhite)
</ins><span class="cx"> {
</span><span class="cx">     ASSERT(!isCompilationThread());
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructureIDBlobh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/StructureIDBlob.h (209953 => 209954)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/StructureIDBlob.h        2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/JavaScriptCore/runtime/StructureIDBlob.h        2016-12-17 02:41:05 UTC (rev 209954)
</span><span class="lines">@@ -46,7 +46,7 @@
</span><span class="cx">         u.fields.indexingTypeIncludingHistory = indexingTypeIncludingHistory;
</span><span class="cx">         u.fields.type = typeInfo.type();
</span><span class="cx">         u.fields.inlineTypeFlags = typeInfo.inlineTypeFlags();
</span><del>-        u.fields.defaultCellState = CellState::NewWhite;
</del><ins>+        u.fields.defaultCellState = CellState::DefinitelyNewAndWhite;
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     void operator=(const StructureIDBlob&amp; other) { u.doubleWord = other.u.doubleWord; }
</span></span></pre>
</div>
</div>

</body>
</html>