Changeset 48580 in webkit for trunk/JavaScriptCore

Timestamp:

Sep 21, 2009, 7:45:23 AM (16 years ago)

Author:

Adam Roben

Message:

Revert r48573, as it caused many assertion failures

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/Interpreter.cpp (modified) (3 diffs)
• jit/JITStubs.cpp (modified) (6 diffs)
• runtime/BatchedTransitionOptimizer.h (modified) (1 diff)
• runtime/JSObject.cpp (modified) (1 diff)
• runtime/Structure.cpp (modified) (9 diffs)
• runtime/Structure.h (modified) (4 diffs)
• runtime/StructureChain.cpp (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ -18 +18 @@
         (JSC::JITThunks::JITThunks):
         * jit/JITStubs.h:
-
-2009-09-20  Oliver Hunt  <[email protected]>
-
-        Reviewed by Maciej Stachowiak.
-
-        SNES is too slow
-        https://bugs.webkit.org/show_bug.cgi?id=29534
-
-        The problem was that the emulator used multiple classes with
-        more properties than our dictionary cutoff allowed, this resulted
-        in more or less all critical logic inside the emulator requiring
-        uncached property access.
-
-        Rather than simply bumping the dictionary cutoff, this patch
-        recognises that there are two ways to create a "dictionary"
-        structure.  Either by adding a large number of properties, or
-        by removing a property.  In the case of adding properties we
-        know all the existing properties will maintain their existing
-        offsets, so we could cache access to those properties, if we
-        know they won't be removed.
-
-        To make this possible, this patch adds the logic required to
-        distinguish a dictionary created by addition from one created
-        by removal.  With this logic in place we can now cache access
-        to objects with large numbers of properties.
-
-        SNES performance improved by more than 6x.
-
-        * interpreter/Interpreter.cpp:
-        (JSC::Interpreter::resolveGlobal):
-        (JSC::Interpreter::tryCachePutByID):
-        (JSC::Interpreter::tryCacheGetByID):
-        * jit/JITStubs.cpp:
-        (JSC::JITThunks::tryCachePutByID):
-        (JSC::JITThunks::tryCacheGetByID):
-        (JSC::DEFINE_STUB_FUNCTION):
-        * runtime/BatchedTransitionOptimizer.h:
-        (JSC::BatchedTransitionOptimizer::BatchedTransitionOptimizer):
-        * runtime/JSObject.cpp:
-        (JSC::JSObject::removeDirect):
-        * runtime/Structure.cpp:
-        (JSC::Structure::Structure):
-        (JSC::Structure::getEnumerablePropertyNames):
-        (JSC::Structure::despecifyDictionaryFunction):
-        (JSC::Structure::addPropertyTransitionToExistingStructure):
-        (JSC::Structure::addPropertyTransition):
-        (JSC::Structure::removePropertyTransition):
-        (JSC::Structure::toDictionaryTransition):
-        (JSC::Structure::toCacheableDictionaryTransition):
-        (JSC::Structure::toUncacheableDictionaryTransition):
-        (JSC::Structure::fromDictionaryTransition):
-        (JSC::Structure::removePropertyWithoutTransition):
-        * runtime/Structure.h:
-        (JSC::Structure::isDictionary):
-        (JSC::Structure::isUncacheableDictionary):
-        (JSC::Structure::):
-        * runtime/StructureChain.cpp:
-        (JSC::StructureChain::isCacheable):
 
 2009-09-19  Oliver Hunt  <[email protected]>

trunk/JavaScriptCore/interpreter/Interpreter.cpp

@@ -170 +170 @@
     if (globalObject->getPropertySlot(callFrame, ident, slot)) {
         JSValue result = slot.getValue(callFrame, ident);
-        if (slot.isCacheable() && !globalObject->structure()->isUncacheableDictionary() && slot.slotBase() == globalObject) {
+        if (slot.isCacheable() && !globalObject->structure()->isDictionary() && slot.slotBase() == globalObject) {
             if (vPC[4].u.structure)
                 vPC[4].u.structure->deref();
@@ -954 +954 @@
     Structure* structure = baseCell->structure();
 
-    if (structure->isUncacheableDictionary()) {
+    if (structure->isDictionary()) {
         vPC[0] = getOpcode(op_put_by_id_generic);
         return;
@@ -1041 +1041 @@
     Structure* structure = asCell(baseValue)->structure();
 
-    if (structure->isUncacheableDictionary()) {
+    if (structure->isDictionary()) {
         vPC[0] = getOpcode(op_get_by_id_generic);
         return;

trunk/JavaScriptCore/jit/JITStubs.cpp

@@ -680 +680 @@
     Structure* structure = baseCell->structure();
 
-    if (structure->isUncacheableDictionary()) {
+    if (structure->isDictionary()) {
         ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_put_by_id_generic));
         return;
@@ -744 +744 @@
     Structure* structure = baseCell->structure();
 
-    if (structure->isUncacheableDictionary()) {
+    if (structure->isDictionary()) {
         ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
         return;
@@ -1155 +1155 @@
     if (baseValue.isCell()
         && slot.isCacheable()
-        && !(structure = asCell(baseValue)->structure())->isUncacheableDictionary()
+        && !(structure = asCell(baseValue)->structure())->isDictionary()
         && (slotBaseObject = asObject(slot.slotBase()))->getPropertySpecificValue(callFrame, ident, specific)
         && specific
@@ -1229 +1229 @@
     if (baseValue.isCell()
         && slot.isCacheable()
-        && !asCell(baseValue)->structure()->isUncacheableDictionary()
+        && !asCell(baseValue)->structure()->isDictionary()
         && slot.slotBase() == baseValue) {
 
@@ -1300 +1300 @@
     CHECK_FOR_EXCEPTION();
 
-    if (!baseValue.isCell() || !slot.isCacheable() || asCell(baseValue)->structure()->isUncacheableDictionary()) {
+    if (!baseValue.isCell() || !slot.isCacheable() || asCell(baseValue)->structure()->isDictionary()) {
         ctiPatchCallByReturnAddress(callFrame->codeBlock(), STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
         return JSValue::encode(result);
@@ -2189 +2189 @@
     if (globalObject->getPropertySlot(callFrame, ident, slot)) {
         JSValue result = slot.getValue(callFrame, ident);
-        if (slot.isCacheable() && !globalObject->structure()->isUncacheableDictionary() && slot.slotBase() == globalObject) {
+        if (slot.isCacheable() && !globalObject->structure()->isDictionary() && slot.slotBase() == globalObject) {
             GlobalResolveInfo& globalResolveInfo = callFrame->codeBlock()->globalResolveInfo(globalResolveInfoIndex);
             if (globalResolveInfo.structure)

trunk/JavaScriptCore/runtime/BatchedTransitionOptimizer.h

@@ -39 +39 @@
         {
             if (!m_object->structure()->isDictionary())
-                m_object->setStructure(Structure::toCacheableDictionaryTransition(m_object->structure()));
+                m_object->setStructure(Structure::toDictionaryTransition(m_object->structure()));
         }
 

trunk/JavaScriptCore/runtime/JSObject.cpp

@@ -472 +472 @@
 {
     size_t offset;
-    if (m_structure->isUncacheableDictionary()) {
+    if (m_structure->isDictionary()) {
         offset = m_structure->removePropertyWithoutTransition(propertyName);
         if (offset != WTF::notFound)

trunk/JavaScriptCore/runtime/Structure.cpp

@@ -128 +128 @@
     , m_propertyStorageCapacity(JSObject::inlineStorageCapacity)
     , m_offset(noOffset)
-    , m_dictionaryKind(NoneDictionaryKind)
+    , m_isDictionary(false)
     , m_isPinnedPropertyTable(false)
     , m_hasGetterSetterProperties(false)
@@ -291 +291 @@
 void Structure::getEnumerablePropertyNames(ExecState* exec, PropertyNameArray& propertyNames, JSObject* baseObject)
 {
-    bool shouldCache = propertyNames.shouldCache() && !(propertyNames.size() || isDictionary());
+    bool shouldCache = propertyNames.shouldCache() && !(propertyNames.size() || m_isDictionary);
 
     if (shouldCache && m_cachedPropertyNameArrayData) {
@@ -350 +350 @@
     materializePropertyMapIfNecessary();
 
-    ASSERT(isDictionary());
+    ASSERT(m_isDictionary);
     ASSERT(m_propertyTable);
 
@@ -392 +392 @@
 PassRefPtr<Structure> Structure::addPropertyTransitionToExistingStructure(Structure* structure, const Identifier& propertyName, unsigned attributes, JSCell* specificValue, size_t& offset)
 {
-    ASSERT(!structure->isDictionary());
+    ASSERT(!structure->m_isDictionary);
     ASSERT(structure->typeInfo().type() == ObjectType);
 
@@ -406 +406 @@
 PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, const Identifier& propertyName, unsigned attributes, JSCell* specificValue, size_t& offset)
 {
-    ASSERT(!structure->isDictionary());
+    ASSERT(!structure->m_isDictionary);
     ASSERT(structure->typeInfo().type() == ObjectType);
     ASSERT(!Structure::addPropertyTransitionToExistingStructure(structure, propertyName, attributes, specificValue, offset));
 
     if (structure->transitionCount() > s_maxTransitionLength) {
-        RefPtr<Structure> transition = toCacheableDictionaryTransition(structure);
+        RefPtr<Structure> transition = toDictionaryTransition(structure);
         ASSERT(structure != transition);
         offset = transition->put(propertyName, attributes, specificValue);
@@ -455 +455 @@
 PassRefPtr<Structure> Structure::removePropertyTransition(Structure* structure, const Identifier& propertyName, size_t& offset)
 {
-    ASSERT(!structure->isUncacheableDictionary());
-
-    RefPtr<Structure> transition = toUncacheableDictionaryTransition(structure);
+    ASSERT(!structure->m_isDictionary);
+
+    RefPtr<Structure> transition = toDictionaryTransition(structure);
 
     offset = transition->remove(propertyName);
@@ -555 +555 @@
 }
 
-PassRefPtr<Structure> Structure::toDictionaryTransition(Structure* structure, DictionaryKind kind)
-{
-    ASSERT(!structure->isDictionary());
-    
+PassRefPtr<Structure> Structure::toDictionaryTransition(Structure* structure)
+{
+    ASSERT(!structure->m_isDictionary);
+
     RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo());
-    transition->m_dictionaryKind = kind;
+    transition->m_isDictionary = true;
     transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
     transition->m_hasGetterSetterProperties = structure->m_hasGetterSetterProperties;
-    
+
     structure->materializePropertyMapIfNecessary();
     transition->m_propertyTable = structure->copyPropertyTable();
     transition->m_isPinnedPropertyTable = true;
-    
+
     return transition.release();
 }
 
-PassRefPtr<Structure> Structure::toCacheableDictionaryTransition(Structure* structure)
-{
-    return toDictionaryTransition(structure, CachedDictionaryKind);
-}
-
-PassRefPtr<Structure> Structure::toUncacheableDictionaryTransition(Structure* structure)
-{
-    return toDictionaryTransition(structure, UncachedDictionaryKind);
-}
-
 PassRefPtr<Structure> Structure::fromDictionaryTransition(Structure* structure)
 {
-    ASSERT(structure->isDictionary());
+    ASSERT(structure->m_isDictionary);
 
     // Since dictionary Structures are not shared, and no opcodes specialize
@@ -592 +582 @@
     // deleted offsets vector) before transitioning from dictionary. 
     if (!structure->m_propertyTable || !structure->m_propertyTable->deletedOffsets || structure->m_propertyTable->deletedOffsets->isEmpty())
-        structure->m_dictionaryKind = NoneDictionaryKind;
+        structure->m_isDictionary = false;
 
     return structure;
@@ -611 +601 @@
 size_t Structure::removePropertyWithoutTransition(const Identifier& propertyName)
 {
-    ASSERT(isUncacheableDictionary());
+    ASSERT(m_isDictionary);
 
     materializePropertyMapIfNecessary();

trunk/JavaScriptCore/runtime/Structure.h

@@ -71 +71 @@
         static PassRefPtr<Structure> addAnonymousSlotsTransition(Structure*, unsigned count);
         static PassRefPtr<Structure> getterSetterTransition(Structure*);
-        static PassRefPtr<Structure> toCacheableDictionaryTransition(Structure*);
-        static PassRefPtr<Structure> toUncacheableDictionaryTransition(Structure*);
+        static PassRefPtr<Structure> toDictionaryTransition(Structure*);
         static PassRefPtr<Structure> fromDictionaryTransition(Structure*);
 
@@ -83 +82 @@
         size_t removePropertyWithoutTransition(const Identifier& propertyName);
         void setPrototypeWithoutTransition(JSValue prototype) { m_prototype = prototype; }
-        
-        bool isDictionary() const { return m_dictionaryKind != NoneDictionaryKind; }
-        bool isUncacheableDictionary() const { return m_dictionaryKind == UncachedDictionaryKind; }
+
+        bool isDictionary() const { return m_isDictionary; }
 
         const TypeInfo& typeInfo() const { return m_typeInfo; }
@@ -130 +128 @@
     private:
         Structure(JSValue prototype, const TypeInfo&);
-        
-        typedef enum { 
-            NoneDictionaryKind = 0,
-            CachedDictionaryKind = 1,
-            UncachedDictionaryKind = 2
-        } DictionaryKind;
-        static PassRefPtr<Structure> toDictionaryTransition(Structure*, DictionaryKind);
 
         size_t put(const Identifier& propertyName, unsigned attributes, JSCell* specificValue);
@@ -197 +188 @@
         signed char m_offset;
 
-        unsigned m_dictionaryKind : 2;
+        bool m_isDictionary : 1;
         bool m_isPinnedPropertyTable : 1;
         bool m_hasGetterSetterProperties : 1;

trunk/JavaScriptCore/runtime/StructureChain.cpp

@@ -52 +52 @@
     
     while (m_vector[i]) {
-        // Both classes of dictionary structure may change arbitrarily so we can't cache them
         if (m_vector[i]->isDictionary())
             return false;