Changeset 41232 in webkit for trunk/JavaScriptCore/jit/JITStubs.cpp

Timestamp:

Feb 25, 2009, 3:44:07 PM (16 years ago)

Author:

[email protected]

Message:

JavaScriptCore:

2009-02-25 Geoffrey Garen <​[email protected]>

Reviewed by Maciej Stachowiak.

Fixed <rdar://problem/6611174> REGRESSION (r36701): Unable to select
messages on hotmail (24052)

The bug was that for-in enumeration used a cached prototype chain without
validating that it was up-to-date.

This led me to refactor prototype chain caching so it was easier to work
with and harder to get wrong.

After a bit of inlining, this patch is performance-neutral on SunSpider
and the v8 benchmarks.

• interpreter/Interpreter.cpp: (JSC::Interpreter::tryCachePutByID): (JSC::Interpreter::tryCacheGetByID):
• jit/JITStubs.cpp: (JSC::JITStubs::tryCachePutByID): (JSC::JITStubs::tryCacheGetByID): (JSC::JITStubs::cti_op_get_by_id_proto_list): Use the new refactored goodness. See lines beginning with "-" and smile.

• runtime/JSGlobalObject.h: (JSC::Structure::prototypeForLookup): A shout out to const.

• runtime/JSPropertyNameIterator.h: (JSC::JSPropertyNameIterator::next): We can use a pointer comparison to see if our cached structure chain is equal to the object's structure chain, since in the case of a cache hit, we share references to the same structure chain.

• runtime/Operations.h: (JSC::countPrototypeChainEntriesAndCheckForProxies): Use the new refactored goodness.

• runtime/PropertyNameArray.h: (JSC::PropertyNameArray::PropertyNameArray): (JSC::PropertyNameArray::setShouldCache): (JSC::PropertyNameArray::shouldCache): Renamed "cacheable" to "shouldCache" to communicate that the client is specifying a recommendation, not a capability.

• runtime/Structure.cpp: (JSC::Structure::Structure): No need to initialize a RefPtr. (JSC::Structure::getEnumerablePropertyNames): Moved some code into helper functions.

(JSC::Structure::prototypeChain): New centralized accessor for a prototype
chain. Revalidates on every access, since the objects in the prototype
chain may have mutated.

(JSC::Structure::isValid): Helper function for revalidating a cached
prototype chain.

(JSC::Structure::getEnumerableNamesFromPropertyTable):
(JSC::Structure::getEnumerableNamesFromClassInfoTable): Factored out of
getEnumerablePropertyNames.

• runtime/Structure.h:

• runtime/StructureChain.cpp: (JSC::StructureChain::StructureChain):
• runtime/StructureChain.h: (JSC::StructureChain::create): No need for structureChainsAreEqual, since we use pointer equality now. Refactored StructureChain to make a little more sense and eliminate special cases for null prototypes.

LayoutTests:

2009-02-24 Geoffrey Garen <​[email protected]>

Reviewed by Maciej Stachowiak.

Added a test for <rdar://problem/6611174> REGRESSION (r36701): Unable to
select messages on hotmail (24052)

• fast/js/for-in-cached-expected.txt: Added.
• fast/js/for-in-cached.html: Added.
• fast/js/resources/for-in-cached.js: Added. (forIn):

File:

• trunk/JavaScriptCore/jit/JITStubs.cpp (modified) (5 diffs)

trunk/JavaScriptCore/jit/JITStubs.cpp

@@ -114 +114 @@
     // Structure transition, cache transition info
     if (slot.type() == PutPropertySlot::NewProperty) {
-        StructureChain* chain = structure->cachedPrototypeChain();
-        if (!chain) {
-            chain = cachePrototypeChain(callFrame, structure);
-            if (!chain) {
-                // This happens if someone has manually inserted null into the prototype chain 
-                stubInfo->opcodeID = op_put_by_id_generic;
-                return;
-            }
-        }
-        stubInfo->initPutByIdTransition(structure->previousID(), structure, chain);
-        JIT::compilePutByIdTransition(callFrame->scopeChain()->globalData, codeBlock, stubInfo, structure->previousID(), structure, slot.cachedOffset(), chain, returnAddress);
+        StructureChain* prototypeChain = structure->prototypeChain(callFrame);
+        stubInfo->initPutByIdTransition(structure->previousID(), structure, prototypeChain);
+        JIT::compilePutByIdTransition(callFrame->scopeChain()->globalData, codeBlock, stubInfo, structure->previousID(), structure, slot.cachedOffset(), prototypeChain, returnAddress);
         return;
     }
@@ -206 +198 @@
         // Since we're accessing a prototype in a loop, it's a good bet that it
         // should not be treated as a dictionary.
-        if (slotBaseObject->structure()->isDictionary()) {
-            RefPtr<Structure> transition = Structure::fromDictionaryTransition(slotBaseObject->structure());
-            slotBaseObject->setStructure(transition.release());
-            asCell(baseValue)->structure()->setCachedPrototypeChain(0);
-        }
+        if (slotBaseObject->structure()->isDictionary())
+            slotBaseObject->setStructure(Structure::fromDictionaryTransition(slotBaseObject->structure()));
         
         stubInfo->initGetByIdProto(structure, slotBaseObject->structure());
@@ -224 +213 @@
     }
 
-    StructureChain* chain = structure->cachedPrototypeChain();
-    if (!chain)
-        chain = cachePrototypeChain(callFrame, structure);
-    ASSERT(chain);
-
-    stubInfo->initGetByIdChain(structure, chain);
-
-    JIT::compileGetByIdChain(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, structure, chain, count, slot.cachedOffset(), returnAddress);
+    StructureChain* prototypeChain = structure->prototypeChain(callFrame);
+    stubInfo->initGetByIdChain(structure, prototypeChain);
+    JIT::compileGetByIdChain(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, structure, prototypeChain, count, slot.cachedOffset(), returnAddress);
 }
 
@@ -673 +657 @@
         // Since we're accessing a prototype in a loop, it's a good bet that it
         // should not be treated as a dictionary.
-        if (slotBaseObject->structure()->isDictionary()) {
-            RefPtr<Structure> transition = Structure::fromDictionaryTransition(slotBaseObject->structure());
-            slotBaseObject->setStructure(transition.release());
-            asCell(baseValue)->structure()->setCachedPrototypeChain(0);
-        }
+        if (slotBaseObject->structure()->isDictionary())
+            slotBaseObject->setStructure(Structure::fromDictionaryTransition(slotBaseObject->structure()));
 
         int listIndex;
@@ -687 +668 @@
             ctiPatchCallByReturnAddress(STUB_RETURN_ADDRESS, reinterpret_cast<void*>(cti_op_get_by_id_proto_list_full));
     } else if (size_t count = countPrototypeChainEntriesAndCheckForProxies(callFrame, baseValue, slot)) {
-        StructureChain* chain = structure->cachedPrototypeChain();
-        if (!chain)
-            chain = cachePrototypeChain(callFrame, structure);
-        ASSERT(chain);
-
         int listIndex;
         PolymorphicAccessStructureList* prototypeStructureList = getPolymorphicAccessStructureListSlot(stubInfo, listIndex);
-
-        JIT::compileGetByIdChainList(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, prototypeStructureList, listIndex, structure, chain, count, slot.cachedOffset());
+        JIT::compileGetByIdChainList(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, prototypeStructureList, listIndex, structure, structure->prototypeChain(callFrame), count, slot.cachedOffset());
 
         if (listIndex == (POLYMORPHIC_LIST_CACHE_SIZE - 1))