Changeset 54129 in webkit


Ignore:
Timestamp:
Feb 1, 2010 1:43:01 AM (14 years ago)
Author:
oliver@apple.com
Message:

2010-01-31 Oliver Hunt <​oliver@apple.com>

Reviewed by Maciej Stachowiak.

JSC is failing to propagate anonymous slot count on some transitions
​https://bugs.webkit.org/show_bug.cgi?id=34321

Remove secondary Structure constructor, and make Structure store a copy
of the number of anonymous slots directly so saving an immediate allocation
of a property map for all structures with anonymous storage, which also
avoids the leaked property map on new property transition in the original
version of this patch.

We need to propagate the the anonymous slot count otherwise we can end up
with a structure recording incorrect information about the available and
needed space for property storage, or alternatively incorrectly reusing
some slots.

  • JavaScriptCore.exp:
  • runtime/Structure.cpp: (JSC::Structure::Structure): (JSC::Structure::materializePropertyMap): (JSC::Structure::addPropertyTransition): (JSC::Structure::changePrototypeTransition): (JSC::Structure::despecifyFunctionTransition): (JSC::Structure::getterSetterTransition): (JSC::Structure::toDictionaryTransition): (JSC::Structure::flattenDictionaryStructure): (JSC::Structure::copyPropertyTable): (JSC::Structure::put): (JSC::Structure::remove): (JSC::Structure::insertIntoPropertyMapHashTable): (JSC::Structure::createPropertyMapHashTable):
  • runtime/Structure.h: (JSC::Structure::create): (JSC::Structure::hasAnonymousSlots): (JSC::Structure::anonymousSlotCount):

2010-02-01 Oliver Hunt <​oliver@apple.com>

Reviewed by Maciej Stachowiak.

JSC is failing to propagate anonymous slot count on some transitions
​https://bugs.webkit.org/show_bug.cgi?id=34321

Add test case for modifying DOM objects with anonymous storage.

  • fast/dom/Window/anonymous-slot-with-changes-expected.txt: Added.
  • fast/dom/Window/anonymous-slot-with-changes.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r54123 r54129  
     12010-01-31  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Maciej Stachowiak.
     4
     5        JSC is failing to propagate anonymous slot count on some transitions
     6        https://bugs.webkit.org/show_bug.cgi?id=34321
     7
     8        Remove secondary Structure constructor, and make Structure store a copy
     9        of the number of anonymous slots directly so saving an immediate allocation
     10        of a property map for all structures with anonymous storage, which also
     11        avoids the leaked property map on new property transition in the original
     12        version of this patch.
     13
     14        We need to propagate the the anonymous slot count otherwise we can end up
     15        with a structure recording incorrect information about the available and
     16        needed space for property storage, or alternatively incorrectly reusing
     17        some slots.
     18
     19        * JavaScriptCore.exp:
     20        * runtime/Structure.cpp:
     21        (JSC::Structure::Structure):
     22        (JSC::Structure::materializePropertyMap):
     23        (JSC::Structure::addPropertyTransition):
     24        (JSC::Structure::changePrototypeTransition):
     25        (JSC::Structure::despecifyFunctionTransition):
     26        (JSC::Structure::getterSetterTransition):
     27        (JSC::Structure::toDictionaryTransition):
     28        (JSC::Structure::flattenDictionaryStructure):
     29        (JSC::Structure::copyPropertyTable):
     30        (JSC::Structure::put):
     31        (JSC::Structure::remove):
     32        (JSC::Structure::insertIntoPropertyMapHashTable):
     33        (JSC::Structure::createPropertyMapHashTable):
     34        * runtime/Structure.h:
     35        (JSC::Structure::create):
     36        (JSC::Structure::hasAnonymousSlots):
     37        (JSC::Structure::anonymousSlotCount):
     38
    1392010-01-31  Patrick Gansterer  <paroga@paroga.com>
    240

  • trunk/JavaScriptCore/JavaScriptCore.exp

    r54022 r54129  
    293293__ZN3JSC9Structure3getEPKNS_11UStringImplERjRPNS_6JSCellE
    294294__ZN3JSC9Structure40addPropertyTransitionToExistingStructureEPS0_RKNS_10IdentifierEjPNS_6JSCellERm
    295 __ZN3JSC9StructureC1ENS_7JSValueERKNS_8TypeInfoE
     295__ZN3JSC9StructureC1ENS_7JSValueERKNS_8TypeInfoEj
    296296__ZN3JSC9StructureD1Ev
    297297__ZN3JSC9constructEPNS_9ExecStateENS_7JSValueENS_13ConstructTypeERKNS_13ConstructDataERKNS_7ArgListE

  • trunk/JavaScriptCore/runtime/Structure.cpp

    r54100 r54129  
    124124}
    125125
    126 Structure::Structure(JSValue prototype, const TypeInfo& typeInfo)
     126Structure::Structure(JSValue prototype, const TypeInfo& typeInfo, unsigned anonymousSlotCount)
    127127    : m_typeInfo(typeInfo)
    128128    , m_prototype(prototype)
    … …  
    136136    , m_attributesInPrevious(0)
    137137    , m_specificFunctionThrashCount(0)
     138    , m_anonymousSlotCount(anonymousSlotCount)
    138139{
    139140    ASSERT(m_prototype);
    … …  
    272273            rehashPropertyMapHashTable(sizeForKeyCount(m_offset + 1)); // This could be made more efficient by combining with the copy above.
    273274    }
    274 
     275   
     276    m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
    275277    for (ptrdiff_t i = structures.size() - 2; i >= 0; --i) {
    276278        structure = structures[i];
    … …  
    367369    }
    368370
    369     RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo());
     371    RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo(), structure->anonymousSlotCount());
    370372
    371373    transition->m_cachedPrototypeChain = structure->m_cachedPrototypeChain;
    … …  
    380382
    381383    if (structure->m_propertyTable) {
     384        ASSERT(structure->m_propertyTable->anonymousSlotCount == structure->m_anonymousSlotCount);
    382385        if (structure->m_isPinnedPropertyTable)
    383386            transition->m_propertyTable = structure->copyPropertyTable();
    … …  
    398401
    399402    transition->m_offset = offset;
    400 
     403    ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
    401404    structure->table.add(make_pair(propertyName.ustring().rep(), attributes), transition.get(), specificValue);
    402405    return transition.release();
    … …  
    416419PassRefPtr<Structure> Structure::changePrototypeTransition(Structure* structure, JSValue prototype)
    417420{
    418     RefPtr<Structure> transition = create(prototype, structure->typeInfo());
     421    RefPtr<Structure> transition = create(prototype, structure->typeInfo(), structure->anonymousSlotCount());
    419422
    420423    transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
    … …  
    428431    transition->m_propertyTable = structure->copyPropertyTable();
    429432    transition->m_isPinnedPropertyTable = true;
    430 
     433   
     434    ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
    431435    return transition.release();
    432436}
    … …  
    435439{
    436440    ASSERT(structure->m_specificFunctionThrashCount < maxSpecificFunctionThrashCount);
    437     RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo());
     441    RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo(), structure->anonymousSlotCount());
    438442
    439443    transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
    … …  
    454458        ASSERT_UNUSED(removed, removed);
    455459    }
    456 
     460   
     461    ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
    457462    return transition.release();
    458463}
    … …  
    460465PassRefPtr<Structure> Structure::getterSetterTransition(Structure* structure)
    461466{
    462     RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo());
     467    RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo(), structure->anonymousSlotCount());
    463468    transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
    464469    transition->m_hasGetterSetterProperties = transition->m_hasGetterSetterProperties;
    … …  
    471476    transition->m_propertyTable = structure->copyPropertyTable();
    472477    transition->m_isPinnedPropertyTable = true;
    473 
     478   
     479    ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
    474480    return transition.release();
    475481}
    … …  
    479485    ASSERT(!structure->isUncacheableDictionary());
    480486   
    481     RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo());
     487    RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo(), structure->anonymousSlotCount());
    482488    transition->m_dictionaryKind = kind;
    483489    transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
    … …  
    490496    transition->m_isPinnedPropertyTable = true;
    491497   
     498    ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
    492499    return transition.release();
    493500}
    … …  
    523530        // reorder the storage, so we have to copy the current values out
    524531        Vector<JSValue> values(propertyCount);
     532        ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    525533        unsigned anonymousSlotCount = m_propertyTable->anonymousSlotCount;
    526534        for (unsigned i = 0; i < propertyCount; i++) {
    … …  
    613621    if (!m_propertyTable)
    614622        return 0;
    615 
     623   
     624    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    616625    size_t tableSize = PropertyMapHashTable::allocationSize(m_propertyTable->size);
    617626    PropertyMapHashTable* newTable = static_cast<PropertyMapHashTable*>(fastMalloc(tableSize));
    … …  
    755764    if (!m_propertyTable)
    756765        createPropertyMapHashTable();
     766    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    757767
    758768    // FIXME: Consider a fast case for tables with no deleted sentinels.
    … …  
    849859    if (!m_propertyTable)
    850860        return notFound;
    851 
     861   
     862    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    852863#if DUMP_PROPERTYMAP_STATS
    853864    ++numProbes;
    … …  
    913924{
    914925    ASSERT(m_propertyTable);
    915 
     926   
     927    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    916928    unsigned i = entry.key->existingHash();
    917929    unsigned k = 0;
    … …  
    963975    m_propertyTable->size = newTableSize;
    964976    m_propertyTable->sizeMask = newTableSize - 1;
     977    m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
    965978
    966979    checkConsistency();

  • trunk/JavaScriptCore/runtime/Structure.h

    r54100 r54129  
    6363        static PassRefPtr<Structure> create(JSValue prototype, const TypeInfo& typeInfo, unsigned anonymousSlotCount)
    6464        {
    65             Structure* structure = (new Structure(prototype, typeInfo));
    66             if (anonymousSlotCount) {
    67                 structure->materializePropertyMap();
    68                 structure->m_isPinnedPropertyTable = true;
    69                 structure->m_propertyTable->anonymousSlotCount = anonymousSlotCount;
    70                 // Currently we don't allow more anonymous slots than fit in the inline capacity
    71                 ASSERT(structure->propertyStorageSize() <= structure->propertyStorageCapacity());
    72             }
    73             return adoptRef(structure);
     65            return adoptRef(new Structure(prototype, typeInfo, anonymousSlotCount));
    7466        }
    7567
    … …  
    135127        bool hasNonEnumerableProperties() const { return m_hasNonEnumerableProperties; }
    136128
    137         bool hasAnonymousSlots() const { return m_propertyTable && m_propertyTable->anonymousSlotCount; }
    138         unsigned anonymousSlotCount() const { return m_propertyTable ? m_propertyTable->anonymousSlotCount : 0; }
     129        bool hasAnonymousSlots() const { return !!m_anonymousSlotCount; }
     130        unsigned anonymousSlotCount() const { return m_anonymousSlotCount; }
    139131       
    140132        bool isEmpty() const { return m_propertyTable ? !m_propertyTable->keyCount : m_offset == noOffset; }
    … …  
    148140       
    149141    private:
    150         static PassRefPtr<Structure> create(JSValue prototype, const TypeInfo& typeInfo)
    151         {
    152             return adoptRef(new Structure(prototype, typeInfo));
    153         }
    154 
    155         Structure(JSValue prototype, const TypeInfo&);
     142
     143        Structure(JSValue prototype, const TypeInfo&, unsigned anonymousSlotCount);
    156144       
    157145        typedef enum {
    … …  
    232220#endif
    233221        unsigned m_specificFunctionThrashCount : 2;
    234         // 10 free bits
     222        unsigned m_anonymousSlotCount : 5;
     223        // 5 free bits
    235224    };
    236225

  • trunk/LayoutTests/ChangeLog

    r54121 r54129  
     12010-02-01  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Maciej Stachowiak.
     4
     5        JSC is failing to propagate anonymous slot count on some transitions
     6        https://bugs.webkit.org/show_bug.cgi?id=34321
     7
     8        Add test case for modifying DOM objects with anonymous storage.
     9
     10        * fast/dom/Window/anonymous-slot-with-changes-expected.txt: Added.
     11        * fast/dom/Window/anonymous-slot-with-changes.html: Added.
     12
    1132010-01-31  Kent Tamura  <tkent@chromium.org>
    214
Note: See TracChangeset for help on using the changeset viewer.