Bug 648801 (new DOM list bindings) - Don't relookup .item() every time. r=bz/jst/mrbkap.
authorAndreas Gal <gal@mozilla.com>
Mon, 23 May 2011 17:39:25 +0200
changeset 78420 bf437c634fda523fe5f9eea88ed99a446fe5eefc
parent 78419 73afd09ad56a73910a7242ab2a00cda1eb2f0eb9
child 78421 dc150e59693ac23aa3d2712369297bc6fa71c8c9
push id21294
push userpvanderbeken@mozilla.com
push dateMon, 10 Oct 2011 08:58:09 +0000
treeherdermozilla-central@432f3a96bc2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, jst, mrbkap
bugs648801
milestone10.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 648801 (new DOM list bindings) - Don't relookup .item() every time. r=bz/jst/mrbkap.
js/src/xpconnect/src/dombindings.cpp
js/src/xpconnect/src/dombindings.h
--- a/js/src/xpconnect/src/dombindings.cpp
+++ b/js/src/xpconnect/src/dombindings.cpp
@@ -88,18 +88,32 @@ NodeList::getProtoShape(JSObject *obj)
 
 void
 NodeList::setProtoShape(JSObject *obj, uint32 shape)
 {
     JS_ASSERT(js::IsProxy(obj) && js::GetProxyHandler(obj) == &NodeList::instance);
     js::SetProxyExtra(obj, 0, PrivateUint32Value(shape));
 }
 
+JSObject *
+NodeList::getItemFunction(JSObject *obj)
+{
+    JS_ASSERT(js::IsProxy(obj) && js::GetProxyHandler(obj) == &NodeList::instance);
+    return &js::GetProxyExtra(obj, 1).toObject();
+}
+
+void
+NodeList::setItemFunction(JSObject *obj, JSObject *funobj)
+{
+    JS_ASSERT(js::IsProxy(obj) && js::GetProxyHandler(obj) == &NodeList::instance);
+    js::SetProxyExtra(obj, 1, ObjectValue(*funobj));
+}
+
 bool
-NodeList::InstanceIsNodeListObject(JSContext *cx, JSObject *obj)
+NodeList::instanceIsNodeListObject(JSContext *cx, JSObject *obj)
 {
     if (!js::IsProxy(obj) || (js::GetProxyHandler(obj) != &NodeList::instance)) {
         // FIXME: Throw a proper DOM exception.
         JS_ReportError(cx, "type error: wrong object");
         return false;
     }
     return true;
 }
@@ -114,30 +128,30 @@ WrapObject(JSContext *cx, JSObject *scop
     qsObjectHelper helper(result, cache);
     return xpc_qsXPCOMObjectToJsval(lccx, helper, &NS_GET_IID(nsIDOMNode),
                                     &interfaces[k_nsIDOMNode], vp);
 }
 
 JSBool
 NodeList::length_getter(JSContext *cx, JSObject *obj, jsid id, Value *vp)
 {
-    if (!InstanceIsNodeListObject(cx, obj))
+    if (!instanceIsNodeListObject(cx, obj))
         return false;
     PRUint32 length;
     getNodeList(obj)->GetLength(&length);
     JS_ASSERT(int32(length) >= 0);
     vp->setInt32(length);
     return true;
 }
 
 JSBool
 NodeList::item(JSContext *cx, uintN argc, jsval *vp)
 {
     JSObject *obj = JS_THIS_OBJECT(cx, vp);
-    if (!obj || !InstanceIsNodeListObject(cx, obj))
+    if (!obj || !instanceIsNodeListObject(cx, obj))
         return false;
     if (argc < 1)
         return xpc_qsThrow(cx, NS_ERROR_XPC_NOT_ENOUGH_ARGS);
     jsval *argv = JS_ARGV(cx, vp);
     uint32 u;
     if (!JS_ValueToECMAUint32(cx, argv[0], &u))
         return false;
     nsINodeList *nodeList = getNodeList(obj);
@@ -171,19 +185,19 @@ NodeList::getPrototype(JSContext *cx)
 }
 
 JSObject *
 NodeList::create(JSContext *cx, nsINodeList *aNodeList)
 {
     JSObject *proto = getPrototype(cx);
     if (!proto)
         return NULL;
-    JSObject *obj = js::NewProxyObject(cx, &NodeList::instance,
-                                       PrivateValue(aNodeList),
-                                       proto, NULL);
+    JSObject *obj = NewProxyObject(cx, &NodeList::instance,
+                                   PrivateValue(aNodeList),
+                                   proto, NULL);
     if (!obj)
         return NULL;
 
     NS_ADDREF(aNodeList);
     setProtoShape(obj, -1);
 
     return obj;
 }
@@ -230,17 +244,17 @@ bool
 NodeList::defineProperty(JSContext *cx, JSObject *proxy, jsid id,
                          PropertyDescriptor *desc)
 {
     // FIXME: expandos
     return true;
 }
 
 bool
-NodeList::getOwnPropertyNames(JSContext *cx, JSObject *proxy, js::AutoIdVector &props)
+NodeList::getOwnPropertyNames(JSContext *cx, JSObject *proxy, AutoIdVector &props)
 {
     // FIXME: expandos
     PRUint32 length;
     getNodeList(proxy)->GetLength(&length);
     JS_ASSERT(int32(length) >= 0);
     for (int32 i = 0; i < int32(length); ++i) {
         if (!props.append(INT_TO_JSID(i)))
             return false;
@@ -251,17 +265,17 @@ NodeList::getOwnPropertyNames(JSContext 
 bool
 NodeList::delete_(JSContext *cx, JSObject *proxy, jsid id, bool *bp)
 {
     // FIXME: expandos
     return true;
 }
 
 bool
-NodeList::enumerate(JSContext *cx, JSObject *proxy, js::AutoIdVector &props)
+NodeList::enumerate(JSContext *cx, JSObject *proxy, AutoIdVector &props)
 {
     // FIXME: enumerate proto as well
     return getOwnPropertyNames(cx, proxy, props);
 }
 
 bool
 NodeList::fix(JSContext *cx, JSObject *proxy, Value *vp)
 {
@@ -294,83 +308,112 @@ NodeList::has(JSContext *cx, JSObject *p
     JSBool found;
     if (!JS_HasPropertyById(cx, js::GetObjectProto(proxy), id, &found))
         return false;
     *bp = !!found;
     return true;
 }
 
 bool
-NodeList::get(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, js::Value *vp)
+NodeList::cacheItemAndLength(JSContext *cx, JSObject *proxy, JSObject *proto)
+{
+    JSPropertyDescriptor desc;
+    if (!JS_GetPropertyDescriptorById(cx, proto, nsDOMClassInfo::sLength_id, JSRESOLVE_QUALIFIED, &desc))
+        return false;
+    if (desc.obj != proto || desc.getter != length_getter)
+        return true; // don't cache
+    if (!JS_GetPropertyDescriptorById(cx, proto, nsDOMClassInfo::sItem_id, JSRESOLVE_QUALIFIED, &desc))
+        return false;
+    if (desc.obj != proto || desc.getter || JSVAL_IS_PRIMITIVE(desc.value) ||
+        !JS_IsNativeFunction(JSVAL_TO_OBJECT(desc.value), item)) {
+        return true; // don't cache
+    }
+    setProtoShape(proxy, js::GetObjectShape(proto));
+    setItemFunction(proxy, JSVAL_TO_OBJECT(desc.value));
+    return true;
+}
+
+bool
+NodeList::checkForCacheHit(JSContext *cx, JSObject *proxy, JSObject *receiver, JSObject *proto,
+                           jsid id, Value *vp, bool *hitp)
+{
+    if (getProtoShape(proxy) != js::GetObjectShape(proto)) {
+        if (!cacheItemAndLength(cx, proxy, proto))
+            return false;
+        if (getProtoShape(proxy) != js::GetObjectShape(proto)) {
+            *hitp = false;
+            return JS_GetPropertyById(cx, proto, id, vp);
+        }
+    }
+    *hitp = true;
+    return true;
+}
+
+bool
+NodeList::get(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, Value *vp)
 {
     // FIXME: expandos
     int32 index;
     if (JSID_IS_INT(id) && ((index = JSID_TO_INT(id)) >= 0)) {
         nsINodeList *nodeList = getNodeList(proxy);
         nsIContent *result = nodeList->GetNodeAt(PRUint32(index));
         if (result)
             return WrapObject(cx, proxy, result, vp);
     }
 
     JSObject *proto = js::GetObjectProto(proxy);
-
     if (id == nsDOMClassInfo::sLength_id) {
-        uint32 kshape = getProtoShape(proxy);
-        uint32 pshape = js::GetObjectShape(proto);
-        do {
-            if (kshape != pshape) {
-                JSPropertyDescriptor desc;
-                if (!JS_GetPropertyDescriptorById(cx, proto, id,
-                                                  JSRESOLVE_QUALIFIED,
-                                                  &desc)) {
-                    return false;
-                }
-                if (desc.obj == proto &&
-                    desc.getter == length_getter) {
-                    setProtoShape(proxy, pshape);
-                } else {
-                    break;
-                }
-            }
-
+        bool hit;
+        if (!checkForCacheHit(cx, proxy, receiver, proto, id, vp, &hit))
+            return false;
+        if (hit) {
             PRUint32 length;
             getNodeList(proxy)->GetLength(&length);
             JS_ASSERT(int32(length) >= 0);
             vp->setInt32(length);
             return true;
-        } while (0);
+        }
+    }
+    else if (id == nsDOMClassInfo::sItem_id) {
+        bool hit;
+        if (!checkForCacheHit(cx, proxy, receiver, proto, id, vp, &hit))
+            return false;
+        if (hit) {
+            vp->setObject(*getItemFunction(proxy));
+            return true;
+        }
     }
 
     return JS_GetPropertyById(cx, proto, id, vp);
 }
 
 bool
 NodeList::set(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, bool strict,
-              js::Value *vp)
+              Value *vp)
 {
     // FIXME: expandos
     return true;
 }
 
 bool
-NodeList::keys(JSContext *cx, JSObject *proxy, js::AutoIdVector &props)
+NodeList::keys(JSContext *cx, JSObject *proxy, AutoIdVector &props)
 {
     // FIXME: expandos
     return getOwnPropertyNames(cx, proxy, props);
 }
 
 bool
-NodeList::iterate(JSContext *cx, JSObject *proxy, uintN flags, js::Value *vp)
+NodeList::iterate(JSContext *cx, JSObject *proxy, uintN flags, Value *vp)
 {
     JS_ReportError(cx, "FIXME");
     return false;
 }
 
 bool
-NodeList::hasInstance(JSContext *cx, JSObject *proxy, const js::Value *vp, bool *bp)
+NodeList::hasInstance(JSContext *cx, JSObject *proxy, const Value *vp, bool *bp)
 {
     *bp = vp->isObject() && js::GetObjectClass(&vp->toObject()) == &NodeListProtoClass;
     return true;
 }
 
 JSString *
 NodeList::obj_toString(JSContext *cx, JSObject *proxy)
 {
--- a/js/src/xpconnect/src/dombindings.h
+++ b/js/src/xpconnect/src/dombindings.h
@@ -46,27 +46,35 @@
 #include "nsINodeList.h"
 
 namespace xpc {
 namespace dom {
 
 class NodeList : public js::ProxyHandler {
     static NodeList instance;
 
-    static bool InstanceIsNodeListObject(JSContext *cx, JSObject *obj);
+    static bool instanceIsNodeListObject(JSContext *cx, JSObject *obj);
 
     static JSObject *getPrototype(JSContext *cx);
 
     static nsINodeList *getNodeList(JSObject *obj);
+
     static uint32 getProtoShape(JSObject *obj);
     static void setProtoShape(JSObject *obj, uint32 shape);
 
+    static JSObject *getItemFunction(JSObject *obj);
+    static void setItemFunction(JSObject *obj, JSObject *funobj);
+
     static JSBool length_getter(JSContext *cx, JSObject *obj, jsid id, js::Value *vp);
 
     static JSBool item(JSContext *cx, uintN argc, jsval *vp);
+
+    static bool cacheItemAndLength(JSContext *cx, JSObject *proxy, JSObject *proto);
+    static bool checkForCacheHit(JSContext *cx, JSObject *proxy, JSObject *receiver, JSObject *proto,
+                                 jsid id, js::Value *vp, bool *hitp);
   public:
     NodeList();
 
     bool getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, bool set,
                                js::PropertyDescriptor *desc);
     bool getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, bool set,
                                   js::PropertyDescriptor *desc);
     bool defineProperty(JSContext *cx, JSObject *proxy, jsid id,