Bug 1507433 - Avoid shape teleporting if any uncacheable prototypes. r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Tue, 27 Nov 2018 13:10:49 +0000
changeset 504738 40e1670547263c1b86b6f58dd4f481830eecd10c
parent 504737 04d915d32eea9361457c6e7b77ebe7341c6e505f
child 504739 614d2236358a744b6650d3f1b6f3b9c30a0357a9
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1507433
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1507433 - Avoid shape teleporting if any uncacheable prototypes. r=jandem These cases are rare and uncacheable prototype shapes are tricky to get right so simplify code instead. The impact is that accessing non-own properties of an object that mutates its prototype will have a few more shape / group guards than are strictly needed. Differential Revision: https://phabricator.services.mozilla.com/D12806
js/src/jit-test/tests/cacheir/shape-teleporting-2.js
js/src/jit/CacheIR.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/cacheir/shape-teleporting-2.js
@@ -0,0 +1,47 @@
+function A(name) { this.name = name; }
+function B() { }
+function C() { }
+
+B.prototype = A0 = new A("0");
+C.prototype = B0 = new B();
+
+var A1 = new A("1");
+var A2 = new A("2");
+
+var B1 = new B();
+var B2 = new B();
+
+var C1 = new C();
+var C2 = new C();
+
+// Object <-+- A0 <-+- B0 <-+
+//          |       |       |
+//          +- A1   +- B1   +- C1
+//          |       |       |
+//          +- A2   +- B2   +- C2
+
+Object.setPrototypeOf(C1, B1);
+Object.setPrototypeOf(C2, B2);
+
+Object.setPrototypeOf(B1, A1);
+Object.setPrototypeOf(B2, A2);
+
+// Object <-+- A0 <--- B0
+//          |
+//          +- A1 <--- B1 <--- C1
+//          |
+//          +- A2 <--- B2 <--- C2
+
+
+function getName(o) { return o.name; }
+
+// Warm up JIT
+for (var i = 0; i < 100; i++) {
+    getName(C1);
+}
+
+assertEq(B1.name, "1");
+assertEq(B2.name, "2");
+
+assertEq(getName(B1), "1");
+assertEq(getName(B2), "2");
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -711,16 +711,36 @@ GeneratePrototypeGuardsForReceiver(Cache
     } else if (obj->is<TypedObject>()) {
         MOZ_ASSERT(!obj->group()->hasUncacheableProto());
     } else if (obj->is<ProxyObject>()) {
         MOZ_ASSERT(!obj->hasUncacheableProto());
     }
 #endif // DEBUG
 }
 
+static bool
+ProtoChainSupportsTeleporting(JSObject* obj, JSObject* holder)
+{
+    // Any non-delegate should already have been handled since its checks are
+    // always required.
+    MOZ_ASSERT(obj->isDelegate());
+
+    // Prototype chain must have cacheable prototypes to ensure the cached
+    // holder is the current holder.
+    for (JSObject* tmp = obj; tmp != holder; tmp = tmp->staticPrototype()) {
+        if (tmp->hasUncacheableProto()) {
+            return false;
+        }
+    }
+
+    // The holder itself only gets reshaped by teleportation if it is not
+    // marked UNCACHEABLE_PROTO. See: ReshapeForProtoMutation.
+    return !holder->hasUncacheableProto();
+}
+
 static void
 GeneratePrototypeGuards(CacheIRWriter& writer, JSObject* obj, JSObject* holder, ObjOperandId objId)
 {
     // Assuming target property is on |holder|, generate appropriate guards to
     // ensure |holder| is still on the prototype chain of |obj| and we haven't
     // introduced any shadowing definitions.
     //
     // For each item in the proto chain before holder, we must ensure that
@@ -751,28 +771,21 @@ GeneratePrototypeGuards(CacheIRWriter& w
     // change since the lookup of 'x' will stop at B.
     //
     // The second condition we must verify is that the prototype chain was not
     // mutated. The same mechanism as above is used. When the prototype link is
     // changed, we generate a new shape for the object. If the object whose
     // link we are mutating is itself a prototype, we regenerate shapes down
     // the chain. This means the same two shape checks as above are sufficient.
     //
-    // Unfortunately we don't stop there and add further caveats. We may set
-    // the UNCACHEABLE_PROTO flag on the shape of an object to indicate that it
-    // will not generate a new shape if its prototype link is modified. If the
-    // object is itself a prototype we follow the shape chain and regenerate
-    // shapes (if they aren't themselves uncacheable).
-    //
-    // Let's consider the effect of the UNCACHEABLE_PROTO flag on our example:
-    // - D is uncacheable: Add check that D still links to C
-    // - C is uncacheable: Modifying C.__proto__ will still reshape B (if B is
-    //                     not uncacheable)
-    // - B is uncacheable: Add shape check C since B will not reshape OR check
-    //                     proto of D and C
+    // An additional wrinkle is the UNCACHEABLE_PROTO shape flag. This
+    // indicates that the shape no longer implies any specific prototype. As
+    // well, the shape will not be updated by the teleporting optimization.
+    // If any shape from receiver to holder (inclusive) is UNCACHEABLE_PROTO,
+    // we don't apply the optimization.
     //
     // See:
     //  - ReshapeForProtoMutation
     //  - ReshapeForShadowedProp
 
     MOZ_ASSERT(holder);
     MOZ_ASSERT(obj != holder);
 
@@ -783,19 +796,18 @@ GeneratePrototypeGuards(CacheIRWriter& w
         // TestMatchingReceiver does not always ensure the prototype is
         // unchanged, so generate extra guards as needed.
         GeneratePrototypeGuardsForReceiver(writer, obj, objId);
 
         pobj = obj->staticPrototype();
     }
     MOZ_ASSERT(pobj->isDelegate());
 
-    // In the common case, holder has a cacheable prototype and will regenerate
-    // its shape if any (delegate) objects in the proto chain are updated.
-    if (!holder->hasUncacheableProto()) {
+    // If teleporting is supported for this prototype chain, we are done.
+    if (ProtoChainSupportsTeleporting(pobj, holder)) {
         return;
     }
 
     // If already at the holder, no further proto checks are needed.
     if (pobj == holder) {
         return;
     }