- Revision
- 209954
- Author
- fpi...@apple.com
- Date
- 2016-12-16 18:41:05 -0800 (Fri, 16 Dec 2016)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (209953 => 209954)
--- trunk/Source/_javascript_Core/ChangeLog 2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-12-17 02:41:05 UTC (rev 209954)
@@ -1,3 +1,45 @@
+2016-12-16 Filip Pizlo <fpi...@apple.com>
+
+ 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):
+
2016-12-16 Saam Barati <sbar...@apple.com>
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)
Modified: trunk/Source/_javascript_Core/heap/CellState.h (209953 => 209954)
--- trunk/Source/_javascript_Core/heap/CellState.h 2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/_javascript_Core/heap/CellState.h 2016-12-17 02:41:05 UTC (rev 209954)
@@ -30,21 +30,19 @@
namespace JSC {
enum class CellState : uint8_t {
- // 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 "anthracite or black" 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,
+ // 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,
// The object is in eden. During GC, this means that the object has not been marked yet.
- NewWhite = 1,
+ DefinitelyNewAndWhite = 1,
// The object is grey - i.e. it will be scanned.
- Grey = 2,
+ DefinitelyGrey = 2,
};
-static const unsigned blackThreshold = 0; // x <= blackThreshold means x is AnthraciteOrBlack.
+static const unsigned blackThreshold = 0; // x <= blackThreshold means x is PossiblyOldOrBlack.
static const unsigned tautologicalThreshold = 100; // x <= tautologicalThreshold is always true.
inline bool isWithinThreshold(CellState cellState, unsigned threshold)
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (209953 => 209954)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2016-12-17 02:41:05 UTC (rev 209954)
@@ -1011,11 +1011,11 @@
return;
}
// It could be that the object was *just* marked. This means that the collector may set the
- // 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->setCellState(CellState::Grey);
+ // 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->setCellState(CellState::DefinitelyGrey);
m_mutatorMarkStack->append(cell);
}
@@ -2016,7 +2016,7 @@
// In this case, the barrierThreshold is the tautological threshold, so from could still be
// not black. But we can't know for sure until we fire off a fence.
WTF::storeLoadFence();
- if (from->cellState() != CellState::AnthraciteOrBlack)
+ if (from->cellState() != CellState::PossiblyOldOrBlack)
return;
}
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (209953 => 209954)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2016-12-17 02:41:05 UTC (rev 209954)
@@ -204,7 +204,7 @@
JSCell* jsCell = static_cast<JSCell*>(heapCell);
validateCell(jsCell);
- jsCell->setCellState(CellState::Grey);
+ jsCell->setCellState(CellState::DefinitelyGrey);
appendToMarkStack(jsCell);
return;
@@ -266,7 +266,7 @@
// Indicate that the object is grey and that:
// In case of concurrent GC: it's the first time it is grey in this GC cycle.
// In case of eden collection: it's a new object that became grey rather than an old remembered object.
- cell->setCellState(CellState::Grey);
+ cell->setCellState(CellState::DefinitelyGrey);
appendToMarkStack(container, cell);
}
@@ -284,7 +284,7 @@
{
ASSERT(Heap::isMarkedConcurrently(cell));
ASSERT(!cell->isZapped());
- ASSERT(cell->cellState() == CellState::Grey);
+ ASSERT(cell->cellState() == CellState::DefinitelyGrey);
container.noteMarked();
@@ -366,7 +366,7 @@
// not clear to me that it would be correct or profitable to bail here if the object is already
// black.
- cell->setCellState(CellState::AnthraciteOrBlack);
+ cell->setCellState(CellState::PossiblyOldOrBlack);
WTF::storeLoadFence();
Modified: trunk/Source/_javascript_Core/runtime/JSCellInlines.h (209953 => 209954)
--- trunk/Source/_javascript_Core/runtime/JSCellInlines.h 2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/_javascript_Core/runtime/JSCellInlines.h 2016-12-17 02:41:05 UTC (rev 209954)
@@ -41,7 +41,7 @@
namespace JSC {
inline JSCell::JSCell(CreatingEarlyCellTag)
- : m_cellState(CellState::NewWhite)
+ : m_cellState(CellState::DefinitelyNewAndWhite)
{
ASSERT(!isCompilationThread());
}
@@ -51,7 +51,7 @@
, m_indexingTypeAndMisc(structure->indexingTypeIncludingHistory())
, m_type(structure->typeInfo().type())
, m_flags(structure->typeInfo().inlineTypeFlags())
- , m_cellState(CellState::NewWhite)
+ , m_cellState(CellState::DefinitelyNewAndWhite)
{
ASSERT(!isCompilationThread());
}
Modified: trunk/Source/_javascript_Core/runtime/StructureIDBlob.h (209953 => 209954)
--- trunk/Source/_javascript_Core/runtime/StructureIDBlob.h 2016-12-17 01:07:38 UTC (rev 209953)
+++ trunk/Source/_javascript_Core/runtime/StructureIDBlob.h 2016-12-17 02:41:05 UTC (rev 209954)
@@ -46,7 +46,7 @@
u.fields.indexingTypeIncludingHistory = indexingTypeIncludingHistory;
u.fields.type = typeInfo.type();
u.fields.inlineTypeFlags = typeInfo.inlineTypeFlags();
- u.fields.defaultCellState = CellState::NewWhite;
+ u.fields.defaultCellState = CellState::DefinitelyNewAndWhite;
}
void operator=(const StructureIDBlob& other) { u.doubleWord = other.u.doubleWord; }
