bug 488029 - fixing bindname optimization regression from the bug 462734 plus creating js_DeclEnvClass instances together with Call objects. r=brendan
authorIgor Bukanov <igor@mir2.org>
Tue, 14 Apr 2009 12:54:37 +0200
changeset 24915 52f12b8b7d5ff8d224d9265994f96f2a58490faa
parent 24914 658bca06045609e889f21f469ad3cacaeae3cbf3
child 24916 dded8cfa2a35ebfafb7e63c865e4c9757e374c80
push id1267
push userrsayre@mozilla.com
push dateSun, 19 Apr 2009 02:47:24 +0000
reviewersbrendan
bugs488029, 462734
milestone1.9.1b4pre
bug 488029 - fixing bindname optimization regression from the bug 462734 plus creating js_DeclEnvClass instances together with Call objects. r=brendan
js/src/jsfun.cpp
js/src/jsfun.h
js/src/jsobj.cpp
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -585,16 +585,24 @@ JSClass js_ArgumentsClass = {
     NULL,               NULL,
     JS_CLASS_TRACE(args_or_call_trace), NULL
 };
 
 #define JSSLOT_CALLEE                    (JSSLOT_PRIVATE + 1)
 #define JSSLOT_CALL_ARGUMENTS            (JSSLOT_PRIVATE + 2)
 #define CALL_CLASS_FIXED_RESERVED_SLOTS  2
 
+JSClass js_DeclEnvClass = {
+    js_Object_str,
+    JSCLASS_HAS_CACHED_PROTO(JSProto_Object),
+    JS_PropertyStub,  JS_PropertyStub,  JS_PropertyStub,  JS_PropertyStub,
+    JS_EnumerateStub, JS_ResolveStub,   JS_ConvertStub,   JS_FinalizeStub,
+    JSCLASS_NO_OPTIONAL_MEMBERS
+};
+
 JSObject *
 js_GetCallObject(JSContext *cx, JSStackFrame *fp)
 {
     JSObject *callobj;
 
     /* Create a call object for fp only if it lacks one. */
     JS_ASSERT(fp->fun);
     callobj = fp->callobj;
@@ -604,26 +612,44 @@ js_GetCallObject(JSContext *cx, JSStackF
 #ifdef DEBUG
     /* A call object should be a frame's outermost scope chain element.  */
     JSClass *classp = OBJ_GET_CLASS(cx, fp->scopeChain);
     if (classp == &js_WithClass || classp == &js_BlockClass || classp == &js_CallClass)
         JS_ASSERT(OBJ_GET_PRIVATE(cx, fp->scopeChain) != fp);
 #endif
 
     /*
-     * Create the call object, using the frame's enclosing scope as
-     * its parent, and link the call to its stack frame.
+     * Create the call object, using the frame's enclosing scope as its
+     * parent, and link the call to its stack frame. For a named function
+     * expression Call's parent points to an environment object holding
+     * function's name.
      */
-    callobj = js_NewObject(cx, &js_CallClass, NULL, fp->scopeChain, 0);
+    JSObject *parent = fp->scopeChain;
+    JSAtom *lambdaName = (fp->fun->flags & JSFUN_LAMBDA) ? fp->fun->atom : NULL;
+    if (lambdaName) {
+        parent = js_NewObjectWithGivenProto(cx, &js_DeclEnvClass, NULL,
+                                            parent, 0);
+        if (!parent)
+            return JS_FALSE;
+    }
+    callobj = js_NewObject(cx, &js_CallClass, NULL, parent, 0);
     if (!callobj)
         return NULL;
 
     JS_SetPrivate(cx, callobj, fp);
     JS_ASSERT(fp->fun == GET_FUNCTION_PRIVATE(cx, fp->callee));
     STOBJ_SET_SLOT(callobj, JSSLOT_CALLEE, OBJECT_TO_JSVAL(fp->callee));
+    if (lambdaName &&
+        !js_DefineNativeProperty(cx, parent, ATOM_TO_JSID(lambdaName),
+                                 OBJECT_TO_JSVAL(fp->callee), NULL, NULL,
+                                 JSPROP_PERMANENT | JSPROP_READONLY,
+                                 0, 0, NULL)) {
+        return JS_FALSE;
+    }
+
     fp->callobj = callobj;
 
     /*
      * Push callobj on the top of the scope chain, and make it the
      * variables object.
      */
     fp->scopeChain = callobj;
     fp->varobj = callobj;
@@ -863,24 +889,16 @@ js_GetCallVar(JSContext *cx, JSObject *o
 }
 
 static JSBool
 SetCallVar(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
 {
     return CallPropertyOp(cx, obj, id, vp, JSCPK_VAR, JS_TRUE);
 }
 
-JSClass js_DeclEnvClass = {
-    js_Object_str,
-    JSCLASS_HAS_CACHED_PROTO(JSProto_Object),
-    JS_PropertyStub,  JS_PropertyStub,  JS_PropertyStub,  JS_PropertyStub,
-    JS_EnumerateStub, JS_ResolveStub,   JS_ConvertStub,   JS_FinalizeStub,
-    JSCLASS_NO_OPTIONAL_MEMBERS
-};
-
 static JSBool
 call_resolve(JSContext *cx, JSObject *obj, jsval idval, uintN flags,
              JSObject **objp)
 {
     jsval callee;
     JSFunction *fun;
     jsid id;
     JSLocalKind localKind;
@@ -939,40 +957,16 @@ call_resolve(JSContext *cx, JSObject *ob
                                      JSPROP_PERMANENT | JSPROP_SHARED,
                                      0, 0, NULL)) {
             return JS_FALSE;
         }
         *objp = obj;
         return JS_TRUE;
     }
 
-    /*
-     * If fun is a named function expression and id matches its name, resolve
-     * this call object's saved callee function object under that name in obj's
-     * parent declarative environment object.
-     */
-    if ((fun->flags & JSFUN_LAMBDA) && JSID_TO_ATOM(id) == fun->atom) {
-        JSObject *parent = STOBJ_GET_PARENT(obj);
-
-        if (STOBJ_GET_CLASS(parent) != &js_DeclEnvClass) {
-            parent = js_NewObjectWithGivenProto(cx, &js_DeclEnvClass, NULL, parent, 0);
-            if (!parent)
-                return JS_FALSE;
-            STOBJ_SET_PARENT(obj, parent);
-
-            attrs = JSPROP_PERMANENT | JSPROP_READONLY;
-            if (!js_DefineNativeProperty(cx, parent, id, callee, NULL, NULL, attrs, 0, 0, NULL))
-                return JS_FALSE;
-        }
-
-        /* Do not resolve, let normal scope chain search find the name. */
-        JS_ASSERT(!*objp);
-        return JS_TRUE;
-    }
-
     /* Control flow reaches here only if id was not resolved. */
     return JS_TRUE;
 }
 
 static JSBool
 call_convert(JSContext *cx, JSObject *obj, JSType type, jsval *vp)
 {
     JSStackFrame *fp;
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -165,16 +165,17 @@ struct JSFunction {
           (flags) | JSFUN_FAST_NATIVE | JSFUN_STUB_GSOPS | JSFUN_TRACEABLE)
 #else
 # define JS_TN(name,fastcall,nargs,flags,trcinfo)                             \
     JS_FN(name, fastcall, nargs, flags)
 #endif
 
 extern JSClass js_ArgumentsClass;
 extern JS_FRIEND_DATA(JSClass) js_CallClass;
+extern JSClass js_DeclEnvClass;
 
 /* JS_FRIEND_DATA so that VALUE_IS_FUNCTION is callable from the shell. */
 extern JS_FRIEND_DATA(JSClass) js_FunctionClass;
 
 #define HAS_FUNCTION_CLASS(obj) (STOBJ_GET_CLASS(obj) == &js_FunctionClass)
 
 /*
  * NB: jsapi.h and jsobj.h must be included before any call to this macro.
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -4064,79 +4064,99 @@ js_FindPropertyHelper(JSContext *cx, jsi
 
 JS_FRIEND_API(JSBool)
 js_FindProperty(JSContext *cx, jsid id, JSObject **objp, JSObject **pobjp,
                 JSProperty **propp)
 {
     return js_FindPropertyHelper(cx, id, objp, pobjp, propp, NULL) >= 0;
 }
 
-JS_REQUIRES_STACK JSObject *
+/*
+ * We cache property lookup results for JSOP_BIND only for the global object or
+ * for native non-global objects without resolve hooks, see bug 462734.
+ */
+static inline bool
+IsCacheableNonGlobalScope(JSObject *obj)
+{
+    JS_ASSERT(STOBJ_GET_PARENT(obj));
+
+    JSClass *clasp = STOBJ_GET_CLASS(obj);
+    bool cacheable = (clasp == &js_CallClass ||
+                      clasp == &js_BlockClass ||
+                      clasp == &js_DeclEnvClass);
+
+    JS_ASSERT_IF(cacheable, obj->map->ops->lookupProperty == js_LookupProperty);
+    return cacheable;
+}
+
+JSObject *
 js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id,
                       JSPropCacheEntry *entry)
 {
     /*
      * This function should not be called for a global object or from the
      * trace and should have a valid cache entry for native scopeChain.
      */
-    JSObject *parent = OBJ_GET_PARENT(cx, scopeChain);
-    JS_ASSERT(parent);
+    JS_ASSERT(OBJ_GET_PARENT(cx, scopeChain));
     JS_ASSERT(!JS_ON_TRACE(cx));
     JS_ASSERT_IF(OBJ_IS_NATIVE(scopeChain), entry);
 
+    JSObject *obj = scopeChain;
+
     /*
-     * Optimize and cache only for classes that do not have resolve hooks and
-     * where the prototype is used only to implement a shared scope, bug 462734
-     * and bug 487039.
+     * Loop over cacheable objects on the scope chain until we find a
+     * property. We also stop when we reach the global object skipping any
+     * farther checks or lookups. For details see the JSOP_BINDNAME case of
+     * js_Interpret.
      */
-    JSObject *obj = scopeChain;
-    for (int scopeIndex = 0; ; scopeIndex++) {
-        JSClass *clasp = OBJ_GET_CLASS(cx, obj);
-        if (clasp != &js_CallClass && clasp != &js_BlockClass)
-            break;
-
+    for (int scopeIndex = 0; IsCacheableNonGlobalScope(obj); scopeIndex++) {
         JSObject *pobj;
         JSProperty *prop;
-        int protoIndex = js_LookupPropertyWithFlags(cx, obj, id, 0,
+        int protoIndex = js_LookupPropertyWithFlags(cx, obj, id,
+                                                    cx->resolveFlags,
                                                     &pobj, &prop);
         if (protoIndex < 0)
             return NULL;
         if (prop) {
             JS_ASSERT(OBJ_IS_NATIVE(pobj));
-            JS_ASSERT(OBJ_GET_CLASS(cx, pobj) == clasp);
+            JS_ASSERT(OBJ_GET_CLASS(cx, pobj) == OBJ_GET_CLASS(cx, obj));
             js_FillPropertyCache(cx, scopeChain, OBJ_SHAPE(scopeChain),
                                  scopeIndex, protoIndex, pobj,
                                  (JSScopeProperty *) prop, &entry);
             JS_UNLOCK_OBJ(cx, pobj);
             return obj;
         }
 
-        obj = parent;
-        parent = OBJ_GET_PARENT(cx, parent);
-        if (!parent) {
-            /*
-             * Here obj is the global one and we can skip any checks for it,
-             * see comments in the JSOP_BINDNAME case of js_Interpret.
-             */
+        /* Call and other cacheable objects always have a parent. */
+        obj = OBJ_GET_PARENT(cx, obj);
+        if (!OBJ_GET_PARENT(cx, obj))
             return obj;
-        }
     }
 
+    /* Loop until we find a property or reach the global object. */
     do {
         JSObject *pobj;
         JSProperty *prop;
         if (!OBJ_LOOKUP_PROPERTY(cx, obj, id, &pobj, &prop))
             return NULL;
         if (prop) {
             OBJ_DROP_PROPERTY(cx, pobj, prop);
             break;
         }
+
+        /*
+         * We conservatively assume that a resolve hook could mutate the scope
+         * chain during OBJ_LOOKUP_PROPERTY. So we must check if parent is not
+         * null here even if it wasn't before the lookup.
+         */
+        JSObject *parent = OBJ_GET_PARENT(cx, obj);
+        if (!parent)
+            break;
         obj = parent;
-        parent = OBJ_GET_PARENT(cx, parent);
-    } while (parent);
+    } while (OBJ_GET_PARENT(cx, obj));
     return obj;
 }
 
 JSBool
 js_NativeGet(JSContext *cx, JSObject *obj, JSObject *pobj,
              JSScopeProperty *sprop, jsval *vp)
 {
     js_LeaveTraceIfGlobalObject(cx, pobj);