Bug 1090636, part 15 - Optimize away the HasOwnProperty call in SetPropertyByDefining, in the common case. No change in behavior, theoretically. r=efaust.
authorJason Orendorff <jorendorff@mozilla.com>
Wed, 12 Nov 2014 11:46:38 -0600
changeset 219577 7837628feeca50da78296127ee3a14d8145d569c
parent 219576 6a71021584ff322cac4d5371a23a72ec95ae7839
child 219578 0064d8949f0c0cd397696b894a6eeb51107c5315
push id27967
push userryanvm@gmail.com
push dateMon, 15 Dec 2014 18:52:54 +0000
treeherdermozilla-central@5d6e0d038f95 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1090636
milestone37.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 1090636, part 15 - Optimize away the HasOwnProperty call in SetPropertyByDefining, in the common case. No change in behavior, theoretically. r=efaust.
js/src/vm/NativeObject.cpp
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1928,25 +1928,39 @@ MaybeReportUndeclaredVarAssignment(JSCon
  * Finish the [[Set]] by defining a new property on receiver.
  *
  * This implements ES6 draft rev 28, 9.1.9 [[Set]] steps 5.c-f, but it
  * is really old code and there are a few barnacles.
  */
 template <ExecutionMode mode>
 static bool
 SetPropertyByDefining(typename ExecutionModeTraits<mode>::ContextType cxArg,
-                      HandleObject receiver, HandleId id, HandleValue v, bool strict)
+                      HandleNativeObject obj, HandleObject receiver, HandleId id,
+                      HandleValue v, bool strict, bool objHasOwn)
 {
     // Step 5.c-d: Test whether receiver has an existing own property
     // receiver[id]. The spec calls [[GetOwnProperty]]; js::HasOwnProperty is
-    // the same thing except faster in the non-proxy case. (Once
-    // SetPropertyHelper is better-behaved, we will be able to optimize away
-    // even the HasOwnProperty call.)
+    // the same thing except faster in the non-proxy case. Sometimes we can
+    // even optimize away the HasOwnProperty call.
     bool existing;
-    if (mode == ParallelExecution) {
+    if (receiver == obj) {
+        // The common case. The caller has necessarily done a property lookup
+        // on obj and passed us the answer as objHasOwn.
+#ifdef DEBUG
+        // Check that objHasOwn is correct. This could fail if receiver or a
+        // native object on its prototype chain has a nondeterministic resolve
+        // hook. We shouldn't have any that are quite that badly behaved.
+        if (mode == SequentialExecution) {
+            if (!HasOwnProperty(cxArg->asJSContext(), receiver, id, &existing))
+                return false;
+            MOZ_ASSERT(existing == objHasOwn);
+        }
+#endif
+        existing = objHasOwn;
+    } else if (mode == ParallelExecution) {
         // Not the fastest possible implementation, but the fastest possible
         // without refactoring LookupPropertyPure or duplicating code.
         NativeObject *npobj;
         Shape *shape;
         if (!LookupPropertyPure(cxArg, receiver, id, &npobj, &shape))
             return false;
         existing = (npobj == receiver);
     } else {
@@ -2022,31 +2036,31 @@ SetPropertyByDefining(typename Execution
  * FIXME: This should be updated to follow ES6 draft rev 28, section 9.1.9,
  * steps 4.d.i and 5.
  *
  * Note that receiver is not necessarily native.
  */
 template <ExecutionMode mode>
 static bool
 SetNonexistentProperty(typename ExecutionModeTraits<mode>::ContextType cxArg,
-                       HandleObject receiver, HandleId id, baseops::QualifiedBool qualified,
-                       HandleValue v, bool strict)
+                       HandleNativeObject obj, HandleObject receiver, HandleId id,
+                       baseops::QualifiedBool qualified, HandleValue v, bool strict)
 {
     // We should never add properties to lexical blocks.
     MOZ_ASSERT(!receiver->is<BlockObject>());
 
     if (receiver->isUnqualifiedVarObj() && !qualified) {
         if (mode == ParallelExecution)
             return false;
 
         if (!MaybeReportUndeclaredVarAssignment(cxArg->asJSContext(), JSID_TO_STRING(id)))
             return false;
     }
 
-    return SetPropertyByDefining<mode>(cxArg, receiver, id, v, strict);
+    return SetPropertyByDefining<mode>(cxArg, obj, receiver, id, v, strict, false);
 }
 
 /*
  * Set an existing own property obj[index] that's a dense element or typed
  * array element.
  */
 template <ExecutionMode mode>
 static bool
@@ -2234,17 +2248,17 @@ SetExistingProperty(typename ExecutionMo
             // We're setting an accessor property.
             if (mode == ParallelExecution)
                 return false;
             return shape->set(cxArg->asJSContext(), obj, receiver, strict, vp);
         }
     }
 
     // Shadow pobj[id] by defining a new data property receiver[id].
-    return SetPropertyByDefining<mode>(cxArg, receiver, id, vp, strict);
+    return SetPropertyByDefining<mode>(cxArg, obj, receiver, id, vp, strict, obj == pobj);
 }
 
 template <ExecutionMode mode>
 bool
 baseops::SetPropertyHelper(typename ExecutionModeTraits<mode>::ContextType cxArg,
                            HandleNativeObject obj, HandleObject receiver, HandleId id,
                            QualifiedBool qualified, MutableHandleValue vp, bool strict)
 {
@@ -2305,17 +2319,17 @@ baseops::SetPropertyHelper(typename Exec
         //   being resolved.
         // - We're running in parallel mode so we've already done the whole
         //   lookup (in LookupPropertyPure above).
         // What they all have in common is we do not want to keep walking
         // the prototype chain.
         RootedObject proto(cxArg, done ? nullptr : pobj->getProto());
         if (!proto) {
             // Step 4.d.i (and step 5).
-            return SetNonexistentProperty<mode>(cxArg, receiver, id, qualified, vp, strict);
+            return SetNonexistentProperty<mode>(cxArg, obj, receiver, id, qualified, vp, strict);
         }
 
         // Step 4.c.i. If the prototype is also native, this step is a
         // recursive tail call, and we don't need to go through all the
         // plumbing of JSObject::setGeneric; the top of the loop is where
         // we're going to end up anyway. But if pobj is non-native,
         // that optimization would be incorrect.
         if (!proto->isNative()) {
@@ -2325,17 +2339,17 @@ baseops::SetPropertyHelper(typename Exec
             // Unqualified assignments are not specified to go through [[Set]]
             // at all, but they do go through this function. So check for
             // unqualified assignment to a nonexistent global (a strict error).
             if (!qualified) {
                 RootedObject pobj(cxArg);
                 if (!JSObject::lookupGeneric(cxArg->asJSContext(), proto, id, &pobj, &shape))
                     return false;
                 if (!shape) {
-                    return SetNonexistentProperty<mode>(cxArg, receiver, id, qualified, vp,
+                    return SetNonexistentProperty<mode>(cxArg, obj, receiver, id, qualified, vp,
                                                         strict);
                 }
             }
 
             return JSObject::setGeneric(cxArg->asJSContext(), proto, receiver, id, vp,
                                         strict);
         }
         pobj = &proto->as<NativeObject>();