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

Reply via email to