The JIT should cache property lookup misses.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Nov 2014 03:10:13 +0000 (03:10 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Nov 2014 03:10:13 +0000 (03:10 +0000)
<https://webkit.org/b/135578>

Source/JavaScriptCore:

Add support for inline caching of missed property lookups.
Previously this would banish us to C++ slow path.

It's implemented as a simple GetById cache that returns jsUndefined()
as long as the Structure chain check passes. There's no DFG exploitation
of this knowledge in this patch.

Test: js/regress/undefined-property-access.js (~5.5x speedup)

Reviewed by Filip Pizlo.

* bytecode/PolymorphicGetByIdList.h:
* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeForStubInfo):

    Add GetByIdAccess::SimpleMiss so we can communicate to the DFG that
    the access has been cached.

* jit/Repatch.cpp:
(JSC::toString):
(JSC::kindFor):
(JSC::generateByIdStub):
(JSC::tryCacheGetByID):
(JSC::tryBuildGetByIDList):

    Added a GetUndefined stub kind, just a simple "store jsUndefined()" snippet.
    Use this to cache missed lookups, piggybacking mostly on the GetValue kind.

* runtime/PropertySlot.h:
(JSC::PropertySlot::isUnset):

    Exposed the unset state so PropertySlot can communicate that lookup failed.

LayoutTests:

Add a JSRegress test for caching of property lookup misses.
There are three subtests:

    1. Pure speed test.
    2. Test for when a property previously cached as missing suddenly
       appears on the object.
    3. Same as (2), but it appears on the prototype.

The test runs ~5.5x faster with the optimization.

Reviewed by Filip Pizlo.

* js/regress/script-tests/undefined-property-access.js: Added.
(foo):
(bar):
(baz):
* js/regress/undefined-property-access-expected.txt: Added.
* js/regress/undefined-property-access.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@175846 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/js/regress/script-tests/undefined-property-access.js [new file with mode: 0644]
LayoutTests/js/regress/undefined-property-access-expected.txt [new file with mode: 0644]
LayoutTests/js/regress/undefined-property-access.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/GetByIdStatus.cpp
Source/JavaScriptCore/bytecode/PolymorphicGetByIdList.h
Source/JavaScriptCore/jit/Repatch.cpp
Source/JavaScriptCore/runtime/PropertySlot.h

index 642577e..8325414 100644 (file)
@@ -1,3 +1,27 @@
+2014-11-10  Andreas Kling  <akling@apple.com>
+
+        The JIT should cache property lookup misses.
+        <https://webkit.org/b/135578>
+
+        Add a JSRegress test for caching of property lookup misses.
+        There are three subtests:
+
+            1. Pure speed test.
+            2. Test for when a property previously cached as missing suddenly
+               appears on the object.
+            3. Same as (2), but it appears on the prototype.
+
+        The test runs ~5.5x faster with the optimization.
+
+        Reviewed by Filip Pizlo.
+
+        * js/regress/script-tests/undefined-property-access.js: Added.
+        (foo):
+        (bar):
+        (baz):
+        * js/regress/undefined-property-access-expected.txt: Added.
+        * js/regress/undefined-property-access.html: Added.
+
 2014-11-10  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Test that complext and fast text codepaths measure the same width
diff --git a/LayoutTests/js/regress/script-tests/undefined-property-access.js b/LayoutTests/js/regress/script-tests/undefined-property-access.js
new file mode 100644 (file)
index 0000000..7a6a848
--- /dev/null
@@ -0,0 +1,48 @@
+var someGlobal;
+
+// This is a simple speed test. It should go fast.
+
+function foo() {
+    var myObject = {};
+    for (var i = 0; i < 10000000; ++i) {
+        someGlobal = myObject.undefinedProperty;
+    }
+    return someGlobal;
+}
+result = foo();
+if (result != undefined)
+    throw "Bad result: " + result;
+
+// This test checks that a cached property lookup miss doesn't continue to fire when the property suddenly appears on the object.
+
+function bar() {
+    var myObject = {};
+    for (var i = 0; i < 100000000; ++i) {
+        someGlobal = myObject.someProperty;
+        if (i == 50000000)
+            myObject.someProperty = 1;
+    }
+    return someGlobal;
+}
+var result = bar();
+if (result != 1)
+    throw "Bad result: " + result;
+someGlobal = undefined;
+
+// This test checks that a cached property lookup miss doesn't continue to fire when the property suddenly appears on the object's prototype.
+
+function baz() {
+    var myPrototype = {}
+    var myObject = {};
+    myObject.__proto__ = myPrototype;
+    for (var i = 0; i < 100000000; ++i) {
+        someGlobal = myObject.someProperty;
+        if (i == 50000000)
+            myPrototype.someProperty = 2;
+    }
+    return someGlobal;
+}
+var result = baz();
+if (result != 2)
+    throw "Bad result: " + result;
+
diff --git a/LayoutTests/js/regress/undefined-property-access-expected.txt b/LayoutTests/js/regress/undefined-property-access-expected.txt
new file mode 100644 (file)
index 0000000..1951f78
--- /dev/null
@@ -0,0 +1,10 @@
+JSRegress/undefined-property-access
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regress/undefined-property-access.html b/LayoutTests/js/regress/undefined-property-access.html
new file mode 100644 (file)
index 0000000..244c7e7
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="../../resources/regress-pre.js"></script>
+<script src="script-tests/undefined-property-access.js"></script>
+<script src="../../resources/regress-post.js"></script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 8a194ed..8acdb9c 100644 (file)
@@ -1,3 +1,41 @@
+2014-11-10  Andreas Kling  <akling@apple.com>
+
+        The JIT should cache property lookup misses.
+        <https://webkit.org/b/135578>
+
+        Add support for inline caching of missed property lookups.
+        Previously this would banish us to C++ slow path.
+
+        It's implemented as a simple GetById cache that returns jsUndefined()
+        as long as the Structure chain check passes. There's no DFG exploitation
+        of this knowledge in this patch.
+
+        Test: js/regress/undefined-property-access.js (~5.5x speedup)
+
+        Reviewed by Filip Pizlo.
+
+        * bytecode/PolymorphicGetByIdList.h:
+        * bytecode/GetByIdStatus.cpp:
+        (JSC::GetByIdStatus::computeForStubInfo):
+
+            Add GetByIdAccess::SimpleMiss so we can communicate to the DFG that
+            the access has been cached.
+
+        * jit/Repatch.cpp:
+        (JSC::toString):
+        (JSC::kindFor):
+        (JSC::generateByIdStub):
+        (JSC::tryCacheGetByID):
+        (JSC::tryBuildGetByIDList):
+
+            Added a GetUndefined stub kind, just a simple "store jsUndefined()" snippet.
+            Use this to cache missed lookups, piggybacking mostly on the GetValue kind.
+
+        * runtime/PropertySlot.h:
+        (JSC::PropertySlot::isUnset):
+
+            Exposed the unset state so PropertySlot can communicate that lookup failed.
+
 2014-11-10  Michael Saboff  <msaboff@apple.com>
 
         Add scope operand to op_create_lexical_environment
index 113347a..e83ad88 100644 (file)
@@ -189,6 +189,7 @@ GetByIdStatus GetByIdStatus::computeForStubInfo(
                             locker, profiledBlock, *stub->m_callLinkInfo, callExitSiteData));
                     break;
                 }
+                case GetByIdAccess::SimpleMiss:
                 case GetByIdAccess::CustomGetter:
                 case GetByIdAccess::WatchedStub:{
                     // FIXME: It would be totally sweet to support this at some point in the future.
index 265b0ad..4867d43 100644 (file)
@@ -47,7 +47,8 @@ public:
         SimpleStub, // This is a stub.
         WatchedStub,
         Getter,
-        CustomGetter
+        CustomGetter,
+        SimpleMiss,
     };
     
     GetByIdAccess()
index 90d8fae..1396b10 100644 (file)
@@ -228,6 +228,7 @@ static void linkRestoreScratch(LinkBuffer& patchBuffer, bool needToRestoreScratc
 
 enum ByIdStubKind {
     GetValue,
+    GetUndefined,
     CallGetter,
     CallCustomGetter,
     CallSetter,
@@ -239,6 +240,8 @@ static const char* toString(ByIdStubKind kind)
     switch (kind) {
     case GetValue:
         return "GetValue";
+    case GetUndefined:
+        return "GetUndefined";
     case CallGetter:
         return "CallGetter";
     case CallCustomGetter:
@@ -257,6 +260,8 @@ static ByIdStubKind kindFor(const PropertySlot& slot)
 {
     if (slot.isCacheableValue())
         return GetValue;
+    if (slot.isUnset())
+        return GetUndefined;
     if (slot.isCacheableCustom())
         return CallCustomGetter;
     RELEASE_ASSERT(slot.isCacheableGetter());
@@ -301,7 +306,7 @@ static void generateByIdStub(
         static_cast<GPRReg>(stubInfo.patch.valueGPR));
     GPRReg scratchGPR = TempRegisterSet(stubInfo.patch.usedRegisters).getFreeGPR();
     bool needToRestoreScratch = scratchGPR == InvalidGPRReg;
-    RELEASE_ASSERT(!needToRestoreScratch || kind == GetValue);
+    RELEASE_ASSERT(!needToRestoreScratch || (kind == GetValue || kind == GetUndefined));
     
     CCallHelpers stubJit(&exec->vm(), exec->codeBlock());
     if (needToRestoreScratch) {
@@ -361,24 +366,28 @@ static void generateByIdStub(
     }
     
     currStructure->startWatchingPropertyForReplacements(*vm, offset);
-    GPRReg baseForAccessGPR;
-    if (chain) {
-        // We could have clobbered scratchGPR earlier, so we have to reload from baseGPR to get the target.
-        if (loadTargetFromProxy)
-            stubJit.loadPtr(MacroAssembler::Address(baseGPR, JSProxy::targetOffset()), baseForGetGPR);
-        stubJit.move(MacroAssembler::TrustedImmPtr(protoObject), scratchGPR);
-        baseForAccessGPR = scratchGPR;
-    } else {
-        // For proxy objects, we need to do all the Structure checks before moving the baseGPR into 
-        // baseForGetGPR because if we fail any of the checks then we would have the wrong value in baseGPR
-        // on the slow path.
-        if (loadTargetFromProxy)
-            stubJit.move(scratchGPR, baseForGetGPR);
-        baseForAccessGPR = baseForGetGPR;
+    GPRReg baseForAccessGPR = InvalidGPRReg;
+    if (kind != GetUndefined) {
+        if (chain) {
+            // We could have clobbered scratchGPR earlier, so we have to reload from baseGPR to get the target.
+            if (loadTargetFromProxy)
+                stubJit.loadPtr(MacroAssembler::Address(baseGPR, JSProxy::targetOffset()), baseForGetGPR);
+            stubJit.move(MacroAssembler::TrustedImmPtr(protoObject), scratchGPR);
+            baseForAccessGPR = scratchGPR;
+        } else {
+            // For proxy objects, we need to do all the Structure checks before moving the baseGPR into
+            // baseForGetGPR because if we fail any of the checks then we would have the wrong value in baseGPR
+            // on the slow path.
+            if (loadTargetFromProxy)
+                stubJit.move(scratchGPR, baseForGetGPR);
+            baseForAccessGPR = baseForGetGPR;
+        }
     }
 
     GPRReg loadedValueGPR = InvalidGPRReg;
-    if (kind != CallCustomGetter && kind != CallCustomSetter) {
+    if (kind == GetUndefined)
+        stubJit.moveTrustedValue(jsUndefined(), valueRegs);
+    else if (kind != CallCustomGetter && kind != CallCustomSetter) {
         if (kind == GetValue)
             loadedValueGPR = valueRegs.payloadGPR();
         else
@@ -412,7 +421,7 @@ static void generateByIdStub(
     std::unique_ptr<CallLinkInfo> callLinkInfo;
 
     MacroAssembler::Jump success, fail;
-    if (kind != GetValue) {
+    if (kind != GetValue && kind != GetUndefined) {
         // Need to make sure that whenever this call is made in the future, we remember the
         // place that we made it from. It just so happens to be the place that we are at
         // right now!
@@ -733,10 +742,12 @@ static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, con
     // FIXME: Cache property access for immediates.
     if (!baseValue.isCell())
         return GiveUpOnCache;
+
+    if (!slot.isCacheable() && !slot.isUnset())
+        return GiveUpOnCache;
+
     JSCell* baseCell = baseValue.asCell();
     Structure* structure = baseCell->structure();
-    if (!slot.isCacheable())
-        return GiveUpOnCache;
 
     InlineCacheAction action = actionForCell(*vm, baseCell);
     if (action != AttemptToCache)
@@ -783,7 +794,7 @@ static void patchJumpToGetByIdStub(CodeBlock* codeBlock, StructureStubInfo& stub
 static InlineCacheAction tryBuildGetByIDList(ExecState* exec, JSValue baseValue, const Identifier& ident, const PropertySlot& slot, StructureStubInfo& stubInfo)
 {
     if (!baseValue.isCell()
-        || !slot.isCacheable())
+        || (!slot.isCacheable() && !slot.isUnset()))
         return GiveUpOnCache;
 
     JSCell* baseCell = baseValue.asCell();
@@ -808,20 +819,23 @@ static InlineCacheAction tryBuildGetByIDList(ExecState* exec, JSValue baseValue,
         // We cannot do as much inline caching if the registers were not flushed prior to this GetById. In particular,
         // non-Value cached properties require planting calls, which requires registers to have been flushed. Thus,
         // if registers were not flushed, don't do non-Value caching.
-        if (!slot.isCacheableValue())
+        if (!slot.isCacheableValue() && !slot.isUnset())
             return GiveUpOnCache;
     }
-    
-    PropertyOffset offset = slot.cachedOffset();
+
+    PropertyOffset offset = slot.isUnset() ? invalidOffset : slot.cachedOffset();
     StructureChain* prototypeChain = 0;
     size_t count = 0;
     
-    if (slot.slotBase() != baseValue) {
+    if (slot.isUnset() || slot.slotBase() != baseValue) {
         if (typeInfo.prohibitsPropertyCaching() || structure->isDictionary())
             return GiveUpOnCache;
-        
-        count = normalizePrototypeChainForChainAccess(
-            exec, baseValue, slot.slotBase(), ident, offset);
+
+        if (slot.isUnset())
+            count = normalizePrototypeChain(exec, baseCell);
+        else
+            count = normalizePrototypeChainForChainAccess(
+                exec, baseValue, slot.slotBase(), ident, offset);
         if (count == InvalidPrototypeChain)
             return GiveUpOnCache;
         prototypeChain = structure->prototypeChain(exec);
@@ -843,6 +857,8 @@ static InlineCacheAction tryBuildGetByIDList(ExecState* exec, JSValue baseValue,
     GetByIdAccess::AccessType accessType;
     if (slot.isCacheableValue())
         accessType = slot.watchpointSet() ? GetByIdAccess::WatchedStub : GetByIdAccess::SimpleStub;
+    else if (slot.isUnset())
+        accessType = GetByIdAccess::SimpleMiss;
     else if (slot.isCacheableGetter())
         accessType = GetByIdAccess::Getter;
     else
index f2bcea1..09b9b0a 100644 (file)
@@ -78,6 +78,7 @@ public:
     JSValue getValue(ExecState*, unsigned propertyName) const;
 
     bool isCacheable() const { return m_cacheability == CachingAllowed && m_offset != invalidOffset; }
+    bool isUnset() const { return m_propertyType == TypeUnset; }
     bool isValue() const { return m_propertyType == TypeValue; }
     bool isAccessor() const { return m_propertyType == TypeGetter; }
     bool isCustom() const { return m_propertyType == TypeCustom; }