Changeset 96068 in webkit


Ignore:
Timestamp:
Sep 26, 2011 8:24:03 PM (13 years ago)
Author:
ggaren@apple.com
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.

Location:
trunk/Source/JavaScriptCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r96045 r96068  
     12011-09-26  Geoffrey Garen  <ggaren@apple.com>
     2
     3        REGRESSION (r95912): Conservative marking doesn't filter out pointers to
     4        MarkedBlock metadata
     5        https://bugs.webkit.org/show_bug.cgi?id=68860
     6
     7        Reviewed by Oliver Hunt.
     8       
     9        Bencher says no performance change, maybe a 7% speedup on kraken-imaging-darkroom.
     10
     11        * heap/MarkedBlock.h:
     12        (JSC::MarkedBlock::isAtomAligned): Renamed atomMask to atomAlignment mask
     13        because the mask doesn't produce the actual atom number.
     14
     15        (JSC::MarkedBlock::isLiveCell): Testing just for alignment isn't good
     16        enough; we also need to test that a pointer is beyond the metadata section
     17        of a MarkedBlock, to avoid treating random metadata as a JSCell.
     18
    1192011-09-26  Mark Hahnenberg  <mhahnenberg@apple.com>
    220
  • trunk/Source/JavaScriptCore/heap/MarkedBlock.h

    r95914 r96068  
    141141
    142142    private:
    143         static const size_t atomMask = ~(atomSize - 1); // atomSize must be a power of two.
     143        static const size_t atomAlignmentMask = atomSize - 1; // atomSize must be a power of two.
    144144
    145145        enum BlockState { New, FreeListed, Allocated, Marked, Zapped };
     
    179179    inline bool MarkedBlock::isAtomAligned(const void* p)
    180180    {
    181         return !(reinterpret_cast<Bits>(p) & ~atomMask);
     181        return !(reinterpret_cast<Bits>(p) & atomAlignmentMask);
    182182    }
    183183
     
    279279    inline bool MarkedBlock::isLiveCell(const void* p)
    280280    {
    281         if ((atomNumber(p) - firstAtom()) % m_atomsPerCell) // Filters pointers to cell middles.
     281        ASSERT(MarkedBlock::isAtomAligned(p));
     282        size_t atomNumber = this->atomNumber(p);
     283        size_t firstAtom = this->firstAtom();
     284        if (atomNumber < firstAtom) // Filters pointers into MarkedBlock metadata.
     285            return false;
     286        if ((atomNumber - firstAtom) % m_atomsPerCell) // Filters pointers into cell middles.
    282287            return false;
    283288
Note: See TracChangeset for help on using the changeset viewer.