Changeset 95388 in webkit

Timestamp:

Sep 17, 2011 4:33:01 PM (13 years ago)

Author:

fpizlo@apple.com

Message:

method_check should repatch itself if it finds that the new structure(s)
are the result of transitions from the old structure(s)
​https://bugs.webkit.org/show_bug.cgi?id=68294

Reviewed by Gavin Barraclough.

Previously a patched method_check would slow-path to get_by_id. Now it
slow-paths to method_check_update, which attempts to correct the
method_check due to structure transitions before bailing to get_by_id.

This is a 1-2% speed-up on some benchmarks and is not a slow-down
anywhere, leading to a 0.6% speed-up on the Kraken geomean.

• jit/JITPropertyAccess.cpp:

(JSC::JIT::patchMethodCallProto):

• jit/JITStubs.cpp:

(JSC::DEFINE_STUB_FUNCTION):

• jit/JITStubs.h:
• runtime/Structure.h:

(JSC::Structure::transitivelyTransitionedFrom):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• jit/JITPropertyAccess.cpp (modified) (2 diffs)
• jit/JITStubs.cpp (modified) (1 diff)
• jit/JITStubs.h (modified) (1 diff)
• runtime/Structure.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-09-16  Filip Pizlo  <fpizlo@apple.com>
+
+        method_check should repatch itself if it finds that the new structure(s)
+        are the result of transitions from the old structure(s)
+        https://bugs.webkit.org/show_bug.cgi?id=68294
+
+        Reviewed by Gavin Barraclough.
+        
+        Previously a patched method_check would slow-path to get_by_id. Now it
+        slow-paths to method_check_update, which attempts to correct the
+        method_check due to structure transitions before bailing to get_by_id.
+        
+        This is a 1-2% speed-up on some benchmarks and is not a slow-down
+        anywhere, leading to a 0.6% speed-up on the Kraken geomean.
+
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::patchMethodCallProto):
+        * jit/JITStubs.cpp:
+        (JSC::DEFINE_STUB_FUNCTION):
+        * jit/JITStubs.h:
+        * runtime/Structure.h:
+        (JSC::Structure::transitivelyTransitionedFrom):
+
 2011-09-16  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

@@ -1052 +1052 @@
     RepatchBuffer repatchBuffer(codeBlock);
     
-    ASSERT(!methodCallLinkInfo.cachedStructure);
     CodeLocationDataLabelPtr structureLocation = methodCallLinkInfo.cachedStructure.location();
     methodCallLinkInfo.cachedStructure.set(globalData, structureLocation, codeBlock->ownerExecutable(), structure);
@@ -1060 +1059 @@
     methodCallLinkInfo.cachedPrototype.set(globalData, structureLocation.dataLabelPtrAtOffset(patchOffsetMethodCheckProtoObj), codeBlock->ownerExecutable(), proto);
     methodCallLinkInfo.cachedFunction.set(globalData, structureLocation.dataLabelPtrAtOffset(patchOffsetMethodCheckPutFunction), codeBlock->ownerExecutable(), callee);
-    repatchBuffer.relinkCallerToFunction(returnAddress, FunctionPtr(cti_op_get_by_id));
+    repatchBuffer.relinkCallerToFunction(returnAddress, FunctionPtr(cti_op_get_by_id_method_check_update));
 }
 

trunk/Source/JavaScriptCore/jit/JITStubs.cpp

@@ -1558 +1558 @@
     // Revert the get_by_id op back to being a regular get_by_id - allow it to cache like normal, if it needs to.
     ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id));
+    return JSValue::encode(result);
+}
+
+DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id_method_check_update)
+{
+    STUB_INIT_STACK_FRAME(stackFrame);
+
+    CallFrame* callFrame = stackFrame.callFrame;
+    Identifier& ident = stackFrame.args[1].identifier();
+
+    JSValue baseValue = stackFrame.args[0].jsValue();
+    PropertySlot slot(baseValue);
+    JSValue result = baseValue.get(callFrame, ident, slot);
+    CHECK_FOR_EXCEPTION();
+
+    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
+    MethodCallLinkInfo& methodCallLinkInfo = codeBlock->getMethodCallLinkInfo(STUB_RETURN_ADDRESS);
+
+    ASSERT(methodCallLinkInfo.seenOnce());
+
+    // If we successfully got something, then the base from which it is being accessed must
+    // be an object.  (Assertion to ensure asObject() call below is safe, which comes after
+    // an isCacheable() chceck.
+    ASSERT(!slot.isCacheableValue() || slot.slotBase().isObject());
+
+    // Check that:
+    //   * We're dealing with a JSCell,
+    //   * the property is cachable,
+    //   * it's not a dictionary
+    //   * there is a function cached.
+    Structure* structure;
+    JSCell* specific;
+    JSObject* slotBaseObject;
+    if (!(baseValue.isCell()
+          && slot.isCacheableValue()
+          && !(structure = baseValue.asCell()->structure())->isUncacheableDictionary()
+          && (slotBaseObject = asObject(slot.slotBase()))->getPropertySpecificValue(callFrame, ident, specific)
+          && specific
+          )
+        || (slot.slotBase() != structure->prototypeForLookup(callFrame)
+            && slot.slotBase() != baseValue)) {
+        // Revert the get_by_id op back to being a regular get_by_id - allow it to cache like normal, if it needs to.
+        ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id));
+        return JSValue::encode(result);
+    }
+    
+    // Now check if the situation has changed sufficiently that we should bail out of
+    // doing method_check optimizations entirely, or if it changed only slightly, in
+    // which case we can just repatch.
+    
+    JSValue proto = structure->prototypeForLookup(callFrame);
+    
+    bool previousWasProto = methodCallLinkInfo.cachedPrototype.get() != codeBlock->globalObject()->methodCallDummy();
+    bool currentIsProto = slot.slotBase() == proto;
+    
+    JSObject* callee = asObject(specific);
+    
+    if (previousWasProto != currentIsProto
+        || !structure->transitivelyTransitionedFrom(methodCallLinkInfo.cachedStructure.get())
+        || (previousWasProto && !slotBaseObject->structure()->transitivelyTransitionedFrom(methodCallLinkInfo.cachedPrototypeStructure.get()))
+        || specific != methodCallLinkInfo.cachedFunction.get()) {
+        ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id));
+        return JSValue::encode(result);
+    }
+    
+    // It makes sense to simply repatch the method_check.
+    
+    // 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())
+        slotBaseObject->flattenDictionaryObject(callFrame->globalData());
+    
+    // The result fetched should always be the callee!
+    ASSERT(result == JSValue(callee));
+    
+    // Check to see if the function is on the object's prototype. Patch up the code to optimize.
+    if (slot.slotBase() == proto) {
+        JIT::patchMethodCallProto(callFrame->globalData(), codeBlock, methodCallLinkInfo, callee, structure, slotBaseObject, STUB_RETURN_ADDRESS);
+        return JSValue::encode(result);
+    }
+    
+    ASSERT(slot.slotBase() == baseValue);
+    
+    // Since we generate the method-check to check both the structure and a prototype-structure (since this
+    // is the common case) we have a problem - we need to patch the prototype structure check to do something
+    // useful. We could try to nop it out altogether, but that's a little messy, so lets do something simpler
+    // for now. For now it performs a check on a special object on the global object only used for this
+    // purpose. The object is in no way exposed, and as such the check will always pass.
+    JIT::patchMethodCallProto(callFrame->globalData(), codeBlock, methodCallLinkInfo, callee, structure, callFrame->scopeChain()->globalObject->methodCallDummy(), STUB_RETURN_ADDRESS);
     return JSValue::encode(result);
 }

trunk/Source/JavaScriptCore/jit/JITStubs.h

@@ -335 +335 @@
     EncodedJSValue JIT_STUB cti_op_get_by_id_getter_stub(STUB_ARGS_DECLARATION);
     EncodedJSValue JIT_STUB cti_op_get_by_id_method_check(STUB_ARGS_DECLARATION);
+    EncodedJSValue JIT_STUB cti_op_get_by_id_method_check_update(STUB_ARGS_DECLARATION);
     EncodedJSValue JIT_STUB cti_op_get_by_id_proto_fail(STUB_ARGS_DECLARATION);
     EncodedJSValue JIT_STUB cti_op_get_by_id_proto_list(STUB_ARGS_DECLARATION);

trunk/Source/JavaScriptCore/runtime/Structure.h

@@ -134 +134 @@
 
         Structure* previousID() const { ASSERT(structure()->classInfo() == &s_info); return m_previous.get(); }
+        bool transitivelyTransitionedFrom(Structure* structureToFind);
 
         void growPropertyStorageCapacity();
@@ -353 +354 @@
     }
 
+    inline bool Structure::transitivelyTransitionedFrom(Structure* structureToFind)
+    {
+        for (Structure* current = this; current; current = current->previousID()) {
+            if (current == structureToFind)
+                return true;
+        }
+        return false;
+    }
+
 } // namespace JSC