Title: [175846] trunk
Revision
175846
Author
akl...@apple.com
Date
2014-11-10 19:10:13 -0800 (Mon, 10 Nov 2014)

Log Message

The JIT should cache property lookup misses.
<https://webkit.org/b/135578>

Source/_javascript_Core:

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (175845 => 175846)


--- trunk/LayoutTests/ChangeLog	2014-11-11 03:05:25 UTC (rev 175845)
+++ trunk/LayoutTests/ChangeLog	2014-11-11 03:10:13 UTC (rev 175846)
@@ -1,3 +1,27 @@
+2014-11-10  Andreas Kling  <akl...@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  <mmaxfi...@apple.com>
 
         Test that complext and fast text codepaths measure the same width

Added: trunk/LayoutTests/js/regress/script-tests/undefined-property-access.js (0 => 175846)


--- trunk/LayoutTests/js/regress/script-tests/undefined-property-access.js	                        (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/undefined-property-access.js	2014-11-11 03:10:13 UTC (rev 175846)
@@ -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;
+

Added: trunk/LayoutTests/js/regress/undefined-property-access-expected.txt (0 => 175846)


--- trunk/LayoutTests/js/regress/undefined-property-access-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress/undefined-property-access-expected.txt	2014-11-11 03:10:13 UTC (rev 175846)
@@ -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
+

Added: trunk/LayoutTests/js/regress/undefined-property-access.html (0 => 175846)


--- trunk/LayoutTests/js/regress/undefined-property-access.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress/undefined-property-access.html	2014-11-11 03:10:13 UTC (rev 175846)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>

Modified: trunk/Source/_javascript_Core/ChangeLog (175845 => 175846)


--- trunk/Source/_javascript_Core/ChangeLog	2014-11-11 03:05:25 UTC (rev 175845)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-11-11 03:10:13 UTC (rev 175846)
@@ -1,3 +1,41 @@
+2014-11-10  Andreas Kling  <akl...@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  <msab...@apple.com>
 
         Add scope operand to op_create_lexical_environment

Modified: trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp (175845 => 175846)


--- trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp	2014-11-11 03:05:25 UTC (rev 175845)
+++ trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp	2014-11-11 03:10:13 UTC (rev 175846)
@@ -189,6 +189,7 @@
                             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.

Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicGetByIdList.h (175845 => 175846)


--- trunk/Source/_javascript_Core/bytecode/PolymorphicGetByIdList.h	2014-11-11 03:05:25 UTC (rev 175845)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicGetByIdList.h	2014-11-11 03:10:13 UTC (rev 175846)
@@ -47,7 +47,8 @@
         SimpleStub, // This is a stub.
         WatchedStub,
         Getter,
-        CustomGetter
+        CustomGetter,
+        SimpleMiss,
     };
     
     GetByIdAccess()

Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (175845 => 175846)


--- trunk/Source/_javascript_Core/jit/Repatch.cpp	2014-11-11 03:05:25 UTC (rev 175845)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp	2014-11-11 03:10:13 UTC (rev 175846)
@@ -228,6 +228,7 @@
 
 enum ByIdStubKind {
     GetValue,
+    GetUndefined,
     CallGetter,
     CallCustomGetter,
     CallSetter,
@@ -239,6 +240,8 @@
     switch (kind) {
     case GetValue:
         return "GetValue";
+    case GetUndefined:
+        return "GetUndefined";
     case CallGetter:
         return "CallGetter";
     case CallCustomGetter:
@@ -257,6 +260,8 @@
 {
     if (slot.isCacheableValue())
         return GetValue;
+    if (slot.isUnset())
+        return GetUndefined;
     if (slot.isCacheableCustom())
         return CallCustomGetter;
     RELEASE_ASSERT(slot.isCacheableGetter());
@@ -301,7 +306,7 @@
         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 @@
     }
     
     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 @@
     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 @@
     // 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 = "" baseCell);
     if (action != AttemptToCache)
@@ -783,7 +794,7 @@
 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 @@
         // 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 @@
     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

Modified: trunk/Source/_javascript_Core/runtime/PropertySlot.h (175845 => 175846)


--- trunk/Source/_javascript_Core/runtime/PropertySlot.h	2014-11-11 03:05:25 UTC (rev 175845)
+++ trunk/Source/_javascript_Core/runtime/PropertySlot.h	2014-11-11 03:10:13 UTC (rev 175846)
@@ -78,6 +78,7 @@
     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; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to