Title: [205979] trunk/Source/_javascript_Core
- Revision
- 205979
- Author
- [email protected]
- Date
- 2016-09-15 10:17:07 -0700 (Thu, 15 Sep 2016)
Log Message
There is no good reason for WeakBlock to care about newly allocated objects
https://bugs.webkit.org/show_bug.cgi?id=162006
Reviewed by Geoffrey Garen.
WeakBlock scans itself in two modes:
visit: if a Weak in the block belongs to an unmarked object, ask the Weak to consider whether
it should do things.
reap: if a Weak in a block belongs to an unmarked object, delete the Weak.
Except that "unmarked" has a peculiar meaning: WeakBlock defines it as
!markedOrNewlyAllocated. So, a newly allocated object will never be consulted about anything.
That sounds scary until you realize that newlyAllocated must have been cleared before we even
got here.
So, we were paying the price of checking newlyAllocated for no reason. This switches the code
to using isMarked(). I don't know why the code previously checked newlyAllocated, but I do
trust my reasoning.
* heap/LargeAllocation.h:
(JSC::LargeAllocation::isMarkedDuringWeakVisiting):
(JSC::LargeAllocation::isMarkedOrNewlyAllocatedDuringWeakVisiting): Deleted.
* heap/MarkedBlock.h:
(JSC::MarkedBlock::isMarkedDuringWeakVisiting):
(JSC::MarkedBlock::isMarkedOrNewlyAllocatedDuringWeakVisiting): Deleted.
* heap/WeakBlock.cpp:
(JSC::WeakBlock::specializedVisit):
(JSC::WeakBlock::reap):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (205978 => 205979)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-15 17:10:40 UTC (rev 205978)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-09-15 17:17:07 UTC (rev 205979)
@@ -1,3 +1,36 @@
+2016-09-14 Filip Pizlo <[email protected]>
+
+ There is no good reason for WeakBlock to care about newly allocated objects
+ https://bugs.webkit.org/show_bug.cgi?id=162006
+
+ Reviewed by Geoffrey Garen.
+
+ WeakBlock scans itself in two modes:
+
+ visit: if a Weak in the block belongs to an unmarked object, ask the Weak to consider whether
+ it should do things.
+
+ reap: if a Weak in a block belongs to an unmarked object, delete the Weak.
+
+ Except that "unmarked" has a peculiar meaning: WeakBlock defines it as
+ !markedOrNewlyAllocated. So, a newly allocated object will never be consulted about anything.
+ That sounds scary until you realize that newlyAllocated must have been cleared before we even
+ got here.
+
+ So, we were paying the price of checking newlyAllocated for no reason. This switches the code
+ to using isMarked(). I don't know why the code previously checked newlyAllocated, but I do
+ trust my reasoning.
+
+ * heap/LargeAllocation.h:
+ (JSC::LargeAllocation::isMarkedDuringWeakVisiting):
+ (JSC::LargeAllocation::isMarkedOrNewlyAllocatedDuringWeakVisiting): Deleted.
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::isMarkedDuringWeakVisiting):
+ (JSC::MarkedBlock::isMarkedOrNewlyAllocatedDuringWeakVisiting): Deleted.
+ * heap/WeakBlock.cpp:
+ (JSC::WeakBlock::specializedVisit):
+ (JSC::WeakBlock::reap):
+
2016-09-15 Commit Queue <[email protected]>
Unreviewed, rolling out r205931.
Modified: trunk/Source/_javascript_Core/heap/LargeAllocation.h (205978 => 205979)
--- trunk/Source/_javascript_Core/heap/LargeAllocation.h 2016-09-15 17:10:40 UTC (rev 205978)
+++ trunk/Source/_javascript_Core/heap/LargeAllocation.h 2016-09-15 17:17:07 UTC (rev 205979)
@@ -72,7 +72,7 @@
ALWAYS_INLINE bool isMarked() { return m_isMarked.load(std::memory_order_relaxed); }
bool isMarkedOrNewlyAllocated() { return isMarked() || isNewlyAllocated(); }
bool isMarkedOrNewlyAllocated(HeapCell*) { return isMarkedOrNewlyAllocated(); }
- bool isMarkedOrNewlyAllocatedDuringWeakVisiting(HeapVersion, HeapCell*) { return isMarkedOrNewlyAllocated(); }
+ bool isMarkedDuringWeakVisiting(HeapVersion, HeapCell*) { return isMarked(); }
bool isLive() { return isMarkedOrNewlyAllocated(); }
bool hasValidCell() const { return m_hasValidCell; }
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (205978 => 205979)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.h 2016-09-15 17:10:40 UTC (rev 205978)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h 2016-09-15 17:17:07 UTC (rev 205979)
@@ -251,7 +251,8 @@
bool testAndSetMarked(const void*);
bool isMarkedOrNewlyAllocated(const HeapCell*);
- bool isMarkedOrNewlyAllocatedDuringWeakVisiting(HeapVersion, const HeapCell*);
+
+ bool isMarkedDuringWeakVisiting(HeapVersion, const HeapCell*);
bool isAtom(const void*);
void clearMarked(const void*);
@@ -561,11 +562,11 @@
return isMarked(cell) || (m_handle.m_newlyAllocated && m_handle.isNewlyAllocated(cell));
}
-inline bool MarkedBlock::isMarkedOrNewlyAllocatedDuringWeakVisiting(HeapVersion heapVersion, const HeapCell* cell)
+inline bool MarkedBlock::isMarkedDuringWeakVisiting(HeapVersion heapVersion, const HeapCell* cell)
{
if (needsFlip(heapVersion))
return false;
- return isMarkedOrNewlyAllocated(cell);
+ return isMarked(cell);
}
inline bool MarkedBlock::Handle::isLive(const HeapCell* cell)
Modified: trunk/Source/_javascript_Core/heap/WeakBlock.cpp (205978 => 205979)
--- trunk/Source/_javascript_Core/heap/WeakBlock.cpp 2016-09-15 17:10:40 UTC (rev 205978)
+++ trunk/Source/_javascript_Core/heap/WeakBlock.cpp 2016-09-15 17:17:07 UTC (rev 205979)
@@ -113,7 +113,7 @@
continue;
const JSValue& jsValue = weakImpl->jsValue();
- if (container.isMarkedOrNewlyAllocatedDuringWeakVisiting(version, jsValue.asCell()))
+ if (container.isMarkedDuringWeakVisiting(version, jsValue.asCell()))
continue;
if (!weakHandleOwner->isReachableFromOpaqueRoots(Handle<Unknown>::wrapSlot(&const_cast<JSValue&>(jsValue)), weakImpl->context(), visitor))
@@ -157,7 +157,7 @@
if (weakImpl->state() > WeakImpl::Dead)
continue;
- if (m_container.isMarkedOrNewlyAllocated(weakImpl->jsValue().asCell())) {
+ if (m_container.isMarked(weakImpl->jsValue().asCell())) {
ASSERT(weakImpl->state() == WeakImpl::Live);
continue;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
