Bug 1394290 - Optimize Proxy::get/set by avoiding unnecessary receiver checking. r?jandem draft
authorTing-Yu Chou <janus926@gmail.com>
Mon, 28 Aug 2017 16:04:38 +0800
changeset 653997 328a2c9aa997e60febbfa531cff472e73551ba87
parent 653766 d10c97627b51a226e19d0fa801201897fe1932f6
child 728469 b45b709d07541beb44c2c102fb1f642022b9fcfe
push id76469
push userbmo:janus926@gmail.com
push dateMon, 28 Aug 2017 08:16:21 +0000
reviewersjandem
bugs1394290
milestone57.0a1
Bug 1394290 - Optimize Proxy::get/set by avoiding unnecessary receiver checking. r?jandem MozReview-Commit-ID: 7K67zt3JrQl
js/src/proxy/Proxy.cpp
js/src/proxy/Proxy.h
--- a/js/src/proxy/Proxy.cpp
+++ b/js/src/proxy/Proxy.cpp
@@ -295,39 +295,37 @@ js::ProxyHasOwn(JSContext* cx, HandleObj
     if (!Proxy::hasOwn(cx, proxy, id, &hasOwn))
         return false;
 
     result.setBoolean(hasOwn);
     return true;
 }
 
 static MOZ_ALWAYS_INLINE Value
-ValueToWindowProxyIfWindow(const Value& v)
+ValueToWindowProxyIfWindow(const Value& v, JSObject* proxy)
 {
-    if (v.isObject())
+    if (v.isObject() && v != ObjectValue(*proxy))
         return ObjectValue(*ToWindowProxyIfWindow(&v.toObject()));
     return v;
 }
 
 MOZ_ALWAYS_INLINE bool
-Proxy::get(JSContext* cx, HandleObject proxy, HandleValue receiver_, HandleId id,
-           MutableHandleValue vp)
+Proxy::getInternal(JSContext* cx, HandleObject proxy, HandleValue receiver,
+                   HandleId id, MutableHandleValue vp)
 {
+    MOZ_ASSERT_IF(receiver.isObject(), !IsWindow(&receiver.toObject()));
+
     if (!CheckRecursionLimit(cx))
         return false;
     const BaseProxyHandler* handler = proxy->as<ProxyObject>().handler();
     vp.setUndefined(); // default result if we refuse to perform this action
     AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true);
     if (!policy.allowed())
         return policy.returnValue();
 
-    // Use the WindowProxy as receiver if receiver_ is a Window. Proxy handlers
-    // shouldn't have to know about the Window/WindowProxy distinction.
-    RootedValue receiver(cx, ValueToWindowProxyIfWindow(receiver_));
-
     if (handler->hasPrototype()) {
         bool own;
         if (!handler->hasOwn(cx, proxy, id, &own))
             return false;
         if (!own) {
             RootedObject proto(cx);
             if (!GetPrototype(cx, proxy, &proto))
                 return false;
@@ -336,80 +334,98 @@ Proxy::get(JSContext* cx, HandleObject p
             return GetProperty(cx, proto, receiver, id, vp);
         }
     }
 
     return handler->get(cx, proxy, receiver, id, vp);
 }
 
 bool
+Proxy::get(JSContext* cx, HandleObject proxy, HandleValue receiver_, HandleId id,
+           MutableHandleValue vp)
+{
+    // Use the WindowProxy as receiver if receiver_ is a Window. Proxy handlers
+    // shouldn't have to know about the Window/WindowProxy distinction.
+    RootedValue receiver(cx, ValueToWindowProxyIfWindow(receiver_, proxy));
+    return getInternal(cx, proxy, receiver, id, vp);
+}
+
+bool
 js::ProxyGetProperty(JSContext* cx, HandleObject proxy, HandleId id, MutableHandleValue vp)
 {
     RootedValue receiver(cx, ObjectValue(*proxy));
-    return Proxy::get(cx, proxy, receiver, id, vp);
+    return Proxy::getInternal(cx, proxy, receiver, id, vp);
 }
 
 bool
 js::ProxyGetPropertyByValue(JSContext* cx, HandleObject proxy, HandleValue idVal,
                             MutableHandleValue vp)
 {
     RootedId id(cx);
     if (!ValueToId<CanGC>(cx, idVal, &id))
         return false;
 
     RootedValue receiver(cx, ObjectValue(*proxy));
-    return Proxy::get(cx, proxy, receiver, id, vp);
+    return Proxy::getInternal(cx, proxy, receiver, id, vp);
 }
 
-bool
-Proxy::set(JSContext* cx, HandleObject proxy, HandleId id, HandleValue v, HandleValue receiver_,
-           ObjectOpResult& result)
+MOZ_ALWAYS_INLINE bool
+Proxy::setInternal(JSContext* cx, HandleObject proxy, HandleId id, HandleValue v,
+                   HandleValue receiver, ObjectOpResult& result)
 {
+    MOZ_ASSERT_IF(receiver.isObject(), !IsWindow(&receiver.toObject()));
+
     if (!CheckRecursionLimit(cx))
         return false;
     const BaseProxyHandler* handler = proxy->as<ProxyObject>().handler();
     AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true);
     if (!policy.allowed()) {
         if (!policy.returnValue())
             return false;
         return result.succeed();
     }
 
-    // Use the WindowProxy as receiver if receiver_ is a Window. Proxy handlers
-    // shouldn't have to know about the Window/WindowProxy distinction.
-    RootedValue receiver(cx, ValueToWindowProxyIfWindow(receiver_));
-
     // Special case. See the comment on BaseProxyHandler::mHasPrototype.
     if (handler->hasPrototype())
         return handler->BaseProxyHandler::set(cx, proxy, id, v, receiver, result);
 
     return handler->set(cx, proxy, id, v, receiver, result);
 }
 
 bool
+Proxy::set(JSContext* cx, HandleObject proxy, HandleId id, HandleValue v, HandleValue receiver_,
+           ObjectOpResult& result)
+{
+    // Use the WindowProxy as receiver if receiver_ is a Window. Proxy handlers
+    // shouldn't have to know about the Window/WindowProxy distinction.
+    RootedValue receiver(cx, ValueToWindowProxyIfWindow(receiver_, proxy));
+    return setInternal(cx, proxy, id, v, receiver, result);
+}
+
+bool
 js::ProxySetProperty(JSContext* cx, HandleObject proxy, HandleId id, HandleValue val, bool strict)
 {
     ObjectOpResult result;
     RootedValue receiver(cx, ObjectValue(*proxy));
-    if (!Proxy::set(cx, proxy, id, val, receiver, result))
+    if (!Proxy::setInternal(cx, proxy, id, val, receiver, result))
         return false;
     return result.checkStrictErrorOrWarning(cx, proxy, id, strict);
 }
 
 bool
 js::ProxySetPropertyByValue(JSContext* cx, HandleObject proxy, HandleValue idVal, HandleValue val,
                             bool strict)
 {
     RootedId id(cx);
     if (!ValueToId<CanGC>(cx, idVal, &id))
         return false;
 
     ObjectOpResult result;
     RootedValue receiver(cx, ObjectValue(*proxy));
-    if (!Proxy::set(cx, proxy, id, val, receiver, result))
+    if (!Proxy::setInternal(cx, proxy, id, val, receiver, result))
         return false;
     return result.checkStrictErrorOrWarning(cx, proxy, id, strict);
 }
 
 bool
 Proxy::getOwnEnumerablePropertyKeys(JSContext* cx, HandleObject proxy, AutoIdVector& props)
 {
     if (!CheckRecursionLimit(cx))
--- a/js/src/proxy/Proxy.h
+++ b/js/src/proxy/Proxy.h
@@ -37,18 +37,22 @@ class Proxy
     static bool setPrototype(JSContext* cx, HandleObject proxy, HandleObject proto,
                              ObjectOpResult& result);
     static bool getPrototypeIfOrdinary(JSContext* cx, HandleObject proxy, bool* isOrdinary,
                                        MutableHandleObject protop);
     static bool setImmutablePrototype(JSContext* cx, HandleObject proxy, bool* succeeded);
     static bool has(JSContext* cx, HandleObject proxy, HandleId id, bool* bp);
     static bool get(JSContext* cx, HandleObject proxy, HandleValue receiver, HandleId id,
                     MutableHandleValue vp);
+    static bool getInternal(JSContext* cx, HandleObject proxy, HandleValue receiver,
+                            HandleId id, MutableHandleValue vp);
     static bool set(JSContext* cx, HandleObject proxy, HandleId id, HandleValue v,
                     HandleValue receiver, ObjectOpResult& result);
+    static bool setInternal(JSContext* cx, HandleObject proxy, HandleId id, HandleValue v,
+                            HandleValue receiver, ObjectOpResult& result);
     static bool call(JSContext* cx, HandleObject proxy, const CallArgs& args);
     static bool construct(JSContext* cx, HandleObject proxy, const CallArgs& args);
 
     /* SpiderMonkey extensions. */
     static bool getPropertyDescriptor(JSContext* cx, HandleObject proxy, HandleId id,
                                       MutableHandle<JS::PropertyDescriptor> desc);
     static bool hasOwn(JSContext* cx, HandleObject proxy, HandleId id, bool* bp);
     static bool getOwnEnumerablePropertyKeys(JSContext* cx, HandleObject proxy,