Changeset 167501 in webkit


Ignore:
Timestamp:
Apr 18, 2014 1:07:50 PM (10 years ago)
Author:
mhahnenberg@apple.com
Message:

Deleting properties poisons objects
​https://bugs.webkit.org/show_bug.cgi?id=131551

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:
This is ~3% progression on Dromaeo with a ~6% progression on the jslib portion of Dromaeo in particular.

  • runtime/Structure.cpp:

(JSC::Structure::Structure):
(JSC::Structure::materializePropertyMap): We now re-use deleted properties when materializing the property map.
(JSC::Structure::removePropertyTransition): We allow up to 5 deletes for a particular path through the tree of
Structure transitions. After that, we convert to an uncacheable dictionary like we used to. We don't cache
delete transitions, but we allow transitioning from them.
(JSC::Structure::changePrototypeTransition):
(JSC::Structure::despecifyFunctionTransition):
(JSC::Structure::attributeChangeTransition):
(JSC::Structure::toDictionaryTransition):
(JSC::Structure::preventExtensionsTransition):
(JSC::Structure::addPropertyWithoutTransition):
(JSC::Structure::removePropertyWithoutTransition):
(JSC::Structure::pin): Now does only what it says it does--marks the property table as pinned.
(JSC::Structure::pinAndPreventTransitions): More descriptive version of what the old pin() was doing.

  • runtime/Structure.h:
  • runtime/StructureInlines.h:

(JSC::Structure::checkOffsetConsistency): Rearranged variables to be more sensible.

LayoutTests:
New JS regress test. We're ~3.5x faster on this microbenchmark now.

  • js/regress/delete-a-few-properties-then-get-by-id-expected.txt: Added.
  • js/regress/delete-a-few-properties-then-get-by-id.html: Added.
  • js/regress/script-tests/delete-a-few-properties-then-get-by-id.js: Added.

(MyObject):
(foo):

Location:
trunk
Files:
3 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r167500 r167501  
     12014-04-17  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        Deleting properties poisons objects
     4        https://bugs.webkit.org/show_bug.cgi?id=131551
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        New JS regress test. We're ~3.5x faster on this microbenchmark now.
     9
     10        * js/regress/delete-a-few-properties-then-get-by-id-expected.txt: Added.
     11        * js/regress/delete-a-few-properties-then-get-by-id.html: Added.
     12        * js/regress/script-tests/delete-a-few-properties-then-get-by-id.js: Added.
     13        (MyObject):
     14        (foo):
     15
    1162014-04-18  Alexey Proskuryakov  <ap@apple.com>
    217

  • trunk/Source/JavaScriptCore/ChangeLog

    r167467 r167501  
     12014-04-17  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        Deleting properties poisons objects
     4        https://bugs.webkit.org/show_bug.cgi?id=131551
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        This is ~3% progression on Dromaeo with a ~6% progression on the jslib portion of Dromaeo in particular.
     9
     10        * runtime/Structure.cpp:
     11        (JSC::Structure::Structure):
     12        (JSC::Structure::materializePropertyMap): We now re-use deleted properties when materializing the property map.
     13        (JSC::Structure::removePropertyTransition): We allow up to 5 deletes for a particular path through the tree of
     14        Structure transitions. After that, we convert to an uncacheable dictionary like we used to. We don't cache
     15        delete transitions, but we allow transitioning from them.
     16        (JSC::Structure::changePrototypeTransition):
     17        (JSC::Structure::despecifyFunctionTransition):
     18        (JSC::Structure::attributeChangeTransition):
     19        (JSC::Structure::toDictionaryTransition):
     20        (JSC::Structure::preventExtensionsTransition):
     21        (JSC::Structure::addPropertyWithoutTransition):
     22        (JSC::Structure::removePropertyWithoutTransition):
     23        (JSC::Structure::pin): Now does only what it says it does--marks the property table as pinned.
     24        (JSC::Structure::pinAndPreventTransitions): More descriptive version of what the old pin() was doing.
     25        * runtime/Structure.h:
     26        * runtime/StructureInlines.h:
     27        (JSC::Structure::checkOffsetConsistency): Rearranged variables to be more sensible.
     28
    1292014-04-17  Filip Pizlo  <fpizlo@apple.com>
    230

  • trunk/Source/JavaScriptCore/runtime/JSPropertyNameIterator.cpp

    r166792 r167501  
    6464    jsPropertyNameIterator->finishCreation(vm, propertyNames.data(), o);
    6565
    66     if (o->structure()->isDictionary())
     66    // JSPropertyNameIterator doesn't know how to skip deleted buckets, so just give up.
     67    if (o->structure()->isDictionary() || o->structure()->hasDeletedOffsets())
    6768        return jsPropertyNameIterator;
    6869

  • trunk/Source/JavaScriptCore/runtime/Structure.cpp

    r165946 r167501  
    166166    , m_offset(invalidOffset)
    167167    , m_inlineCapacity(inlineCapacity)
     168    , m_forgivenDeletes(0)
    168169    , m_dictionaryKind(NoneDictionaryKind)
    169170    , m_isPinnedPropertyTable(false)
    … …  
    193194    , m_offset(invalidOffset)
    194195    , m_inlineCapacity(0)
     196    , m_forgivenDeletes(0)
    195197    , m_dictionaryKind(NoneDictionaryKind)
    196198    , m_isPinnedPropertyTable(false)
    … …  
    219221    , m_offset(invalidOffset)
    220222    , m_inlineCapacity(previous->m_inlineCapacity)
     223    , m_forgivenDeletes(previous->m_forgivenDeletes)
    221224    , m_dictionaryKind(previous->m_dictionaryKind)
    222225    , m_isPinnedPropertyTable(false)
    … …  
    311314        if (!structure->m_nameInPrevious)
    312315            continue;
    313         PropertyMapEntry entry(vm, this, structure->m_nameInPrevious.get(), structure->m_offset, structure->m_attributesInPrevious, structure->m_specificValueInPrevious.get());
     316
     317        PropertyMapEntry entry(vm, this,
     318            structure->m_nameInPrevious.get(),
     319            propertyTable()->nextOffset(m_inlineCapacity),
     320            structure->m_attributesInPrevious,
     321            structure->m_specificValueInPrevious.get());
    314322        propertyTable()->add(entry, m_offset, PropertyTable::PropertyOffsetMustNotChange);
    315323    }
    … …  
    459467    ASSERT(!structure->isUncacheableDictionary());
    460468
     469    if (structure->m_forgivenDeletes < s_maxForgivenDeletes) {
     470        Structure* transition = create(vm, structure);
     471
     472        DeferGC deferGC(vm.heap);
     473        structure->materializePropertyMapIfNecessary(vm, deferGC);
     474        transition->propertyTable().set(vm, transition, structure->copyPropertyTableForPinning(vm, transition));
     475        transition->m_offset = structure->m_offset;
     476        transition->pinAndPreventTransitions();
     477
     478        offset = transition->remove(propertyName);
     479        ASSERT(offset != invalidOffset);
     480        transition->m_forgivenDeletes = structure->m_forgivenDeletes + 1;
     481
     482        transition->checkOffsetConsistency();
     483        return transition;
     484    }
     485
    461486    Structure* transition = toUncacheableDictionaryTransition(vm, structure);
    462 
    463487    offset = transition->remove(propertyName);
    464 
    465488    transition->checkOffsetConsistency();
    466489    return transition;
    … …  
    477500    transition->propertyTable().set(vm, transition, structure->copyPropertyTableForPinning(vm, transition));
    478501    transition->m_offset = structure->m_offset;
    479     transition->pin();
     502    transition->pinAndPreventTransitions();
    480503
    481504    transition->checkOffsetConsistency();
    … …  
    494517    transition->propertyTable().set(vm, transition, structure->copyPropertyTableForPinning(vm, transition));
    495518    transition->m_offset = structure->m_offset;
    496     transition->pin();
     519    transition->pinAndPreventTransitions();
    497520
    498521    if (transition->m_specificFunctionThrashCount == maxSpecificFunctionThrashCount)
    … …  
    516539        transition->propertyTable().set(vm, transition, structure->copyPropertyTableForPinning(vm, transition));
    517540        transition->m_offset = structure->m_offset;
    518         transition->pin();
     541        transition->pinAndPreventTransitions();
    519542       
    520543        structure = transition;
    … …  
    541564    transition->m_offset = structure->m_offset;
    542565    transition->m_dictionaryKind = kind;
    543     transition->pin();
     566    transition->pinAndPreventTransitions();
    544567
    545568    transition->checkOffsetConsistency();
    … …  
    604627    transition->m_offset = structure->m_offset;
    605628    transition->m_preventExtensions = true;
    606     transition->pin();
     629    transition->pinAndPreventTransitions();
    607630
    608631    transition->checkOffsetConsistency();
    … …  
    753776    materializePropertyMapIfNecessaryForPinning(vm, deferGC);
    754777   
    755     pin();
     778    pinAndPreventTransitions();
    756779
    757780    return putSpecificValue(vm, propertyName, attributes, specificValue);
    … …  
    766789    materializePropertyMapIfNecessaryForPinning(vm, deferGC);
    767790
    768     pin();
     791    pinAndPreventTransitions();
    769792    return remove(propertyName);
    770793}
    … …  
    774797    ASSERT(propertyTable());
    775798    m_isPinnedPropertyTable = true;
     799}
     800
     801void Structure::pinAndPreventTransitions()
     802{
     803    pin();
    776804    clearPreviousID();
    777805    m_nameInPrevious.clear();

  • trunk/Source/JavaScriptCore/runtime/Structure.h

    r167452 r167501  
    138138    void setPrototypeWithoutTransition(VM& vm, JSValue prototype) { m_prototype.set(vm, this, prototype); }
    139139       
     140    bool hasDeletedOffsets() const;
     141
    140142    bool isDictionary() const { return m_dictionaryKind != NoneDictionaryKind; }
    141143    bool isUncacheableDictionary() const { return m_dictionaryKind == UncachedDictionaryKind; }
    … …  
    412414
    413415    WriteBarrier<PropertyTable>& propertyTable();
     416    const WriteBarrier<PropertyTable>& propertyTable() const;
    414417    PropertyTable* takePropertyTableOrCloneIfPinned(VM&, Structure* owner);
    415418    PropertyTable* copyPropertyTable(VM&, Structure* owner);
    … …  
    458461       
    459462    void pin();
     463    void pinAndPreventTransitions();
    460464
    461465    Structure* previous() const
    … …  
    513517    ConcurrentJITLock m_lock;
    514518   
     519    static const unsigned s_maxForgivenDeletes = 5;
     520    unsigned m_forgivenDeletes;
     521
    515522    unsigned m_dictionaryKind : 2;
    516523    bool m_isPinnedPropertyTable : 1;

  • trunk/Source/JavaScriptCore/runtime/StructureInlines.h

    r165676 r167501  
    218218}
    219219
    220 ALWAYS_INLINE WriteBarrier<PropertyTable>& Structure::propertyTable()
     220inline bool Structure::hasDeletedOffsets() const
     221{
     222    // If we had deleted anything then we would have pinned our property table.
     223    if (!propertyTable())
     224        return false;
     225    return propertyTable()->hasDeletedOffset();
     226}
     227
     228inline WriteBarrier<PropertyTable>& Structure::propertyTable()
     229{
     230    return const_cast<WriteBarrier<PropertyTable>&>(static_cast<const Structure*>(this)->propertyTable());
     231}
     232
     233inline const WriteBarrier<PropertyTable>& Structure::propertyTable() const
    221234{
    222235    ASSERT(!globalObject() || !globalObject()->vm().heap.isCollecting());
    … …  
    240253        return true;
    241254   
    242     RELEASE_ASSERT(numberOfSlotsForLastOffset(m_offset, m_inlineCapacity) == propertyTable->propertyStorageSize());
    243255    unsigned totalSize = propertyTable->propertyStorageSize();
     256    RELEASE_ASSERT(numberOfSlotsForLastOffset(m_offset, m_inlineCapacity) == totalSize);
    244257    RELEASE_ASSERT((totalSize < inlineCapacity() ? 0 : totalSize - inlineCapacity()) == numberOfOutOfLineSlotsForLastOffset(m_offset));
    245258
Note: See TracChangeset for help on using the changeset viewer.