- Revision
- 95388
- Author
- fpi...@apple.com
- Date
- 2011-09-17 16:33:01 -0700 (Sat, 17 Sep 2011)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (95387 => 95388)
--- trunk/Source/_javascript_Core/ChangeLog 2011-09-17 23:31:52 UTC (rev 95387)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-09-17 23:33:01 UTC (rev 95388)
@@ -1,3 +1,26 @@
+2011-09-16 Filip Pizlo <fpi...@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 <rn...@webkit.org>
Touch Platform.h in the hope to fix SnowLeopard Intel Release (WebKit2 Tests).
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (95387 => 95388)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2011-09-17 23:31:52 UTC (rev 95387)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2011-09-17 23:33:01 UTC (rev 95388)
@@ -1051,7 +1051,6 @@
{
RepatchBuffer repatchBuffer(codeBlock);
- ASSERT(!methodCallLinkInfo.cachedStructure);
CodeLocationDataLabelPtr structureLocation = methodCallLinkInfo.cachedStructure.location();
methodCallLinkInfo.cachedStructure.set(globalData, structureLocation, codeBlock->ownerExecutable(), structure);
@@ -1059,7 +1058,7 @@
methodCallLinkInfo.cachedPrototypeStructure.set(globalData, structureLocation.dataLabelPtrAtOffset(patchOffsetMethodCheckProtoStruct), codeBlock->ownerExecutable(), prototypeStructure);
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));
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/jit/JITStubs.cpp (95387 => 95388)
--- trunk/Source/_javascript_Core/jit/JITStubs.cpp 2011-09-17 23:31:52 UTC (rev 95387)
+++ trunk/Source/_javascript_Core/jit/JITStubs.cpp 2011-09-17 23:33:01 UTC (rev 95388)
@@ -1560,6 +1560,95 @@
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);
+}
+
DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id)
{
STUB_INIT_STACK_FRAME(stackFrame);
Modified: trunk/Source/_javascript_Core/jit/JITStubs.h (95387 => 95388)
--- trunk/Source/_javascript_Core/jit/JITStubs.h 2011-09-17 23:31:52 UTC (rev 95387)
+++ trunk/Source/_javascript_Core/jit/JITStubs.h 2011-09-17 23:33:01 UTC (rev 95388)
@@ -334,6 +334,7 @@
EncodedJSValue JIT_STUB cti_op_get_by_id_generic(STUB_ARGS_DECLARATION);
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);
EncodedJSValue JIT_STUB cti_op_get_by_id_proto_list_full(STUB_ARGS_DECLARATION);
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (95387 => 95388)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2011-09-17 23:31:52 UTC (rev 95387)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2011-09-17 23:33:01 UTC (rev 95388)
@@ -133,6 +133,7 @@
void visitChildren(SlotVisitor&);
Structure* previousID() const { ASSERT(structure()->classInfo() == &s_info); return m_previous.get(); }
+ bool transitivelyTransitionedFrom(Structure* structureToFind);
void growPropertyStorageCapacity();
unsigned propertyStorageCapacity() const { ASSERT(structure()->classInfo() == &s_info); return m_propertyStorageCapacity; }
@@ -352,6 +353,15 @@
return Hash::Key(structure->m_nameInPrevious.get(), +structure->m_attributesInPrevious);
}
+ inline bool Structure::transitivelyTransitionedFrom(Structure* structureToFind)
+ {
+ for (Structure* current = this; current; current = current->previousID()) {
+ if (current == structureToFind)
+ return true;
+ }
+ return false;
+ }
+
} // namespace JSC
#endif // Structure_h
