Title: [96068] trunk/Source/_javascript_Core
- Revision
- 96068
- Author
- [email protected]
- Date
- 2011-09-26 20:24:03 -0700 (Mon, 26 Sep 2011)
Log Message
REGRESSION (r95912): Conservative marking doesn't filter out pointers to
MarkedBlock metadata
https://bugs.webkit.org/show_bug.cgi?id=68860
Reviewed by Oliver Hunt.
Bencher says no performance change, maybe a 7% speedup on kraken-imaging-darkroom.
* heap/MarkedBlock.h:
(JSC::MarkedBlock::isAtomAligned): Renamed atomMask to atomAlignment mask
because the mask doesn't produce the actual atom number.
(JSC::MarkedBlock::isLiveCell): Testing just for alignment isn't good
enough; we also need to test that a pointer is beyond the metadata section
of a MarkedBlock, to avoid treating random metadata as a JSCell.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (96067 => 96068)
--- trunk/Source/_javascript_Core/ChangeLog 2011-09-27 02:36:55 UTC (rev 96067)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-09-27 03:24:03 UTC (rev 96068)
@@ -1,3 +1,21 @@
+2011-09-26 Geoffrey Garen <[email protected]>
+
+ REGRESSION (r95912): Conservative marking doesn't filter out pointers to
+ MarkedBlock metadata
+ https://bugs.webkit.org/show_bug.cgi?id=68860
+
+ Reviewed by Oliver Hunt.
+
+ Bencher says no performance change, maybe a 7% speedup on kraken-imaging-darkroom.
+
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::isAtomAligned): Renamed atomMask to atomAlignment mask
+ because the mask doesn't produce the actual atom number.
+
+ (JSC::MarkedBlock::isLiveCell): Testing just for alignment isn't good
+ enough; we also need to test that a pointer is beyond the metadata section
+ of a MarkedBlock, to avoid treating random metadata as a JSCell.
+
2011-09-26 Mark Hahnenberg <[email protected]>
Make JSCell::toBoolean non-virtual
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (96067 => 96068)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.h 2011-09-27 02:36:55 UTC (rev 96067)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h 2011-09-27 03:24:03 UTC (rev 96068)
@@ -140,7 +140,7 @@
template <typename Functor> void forEachCell(Functor&);
private:
- static const size_t atomMask = ~(atomSize - 1); // atomSize must be a power of two.
+ static const size_t atomAlignmentMask = atomSize - 1; // atomSize must be a power of two.
enum BlockState { New, FreeListed, Allocated, Marked, Zapped };
@@ -178,7 +178,7 @@
inline bool MarkedBlock::isAtomAligned(const void* p)
{
- return !(reinterpret_cast<Bits>(p) & ~atomMask);
+ return !(reinterpret_cast<Bits>(p) & atomAlignmentMask);
}
inline MarkedBlock* MarkedBlock::blockFor(const void* p)
@@ -278,8 +278,13 @@
inline bool MarkedBlock::isLiveCell(const void* p)
{
- if ((atomNumber(p) - firstAtom()) % m_atomsPerCell) // Filters pointers to cell middles.
+ ASSERT(MarkedBlock::isAtomAligned(p));
+ size_t atomNumber = this->atomNumber(p);
+ size_t firstAtom = this->firstAtom();
+ if (atomNumber < firstAtom) // Filters pointers into MarkedBlock metadata.
return false;
+ if ((atomNumber - firstAtom) % m_atomsPerCell) // Filters pointers into cell middles.
+ return false;
return isLive(static_cast<const JSCell*>(p));
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes