bug 487930 - removal of JSSLOT_ARRAY_LOOKUP_HOLDER. r=mrbkap
authorIgor Bukanov <igor@mir2.org>
Fri, 17 Apr 2009 11:37:59 +0200
changeset 27483 ccb029897983f62640da49670120c84f18b2bfac
parent 27482 10429fdd5346b6f76fb92fce3a3abb438223f1c4
child 27484 6e609b154f7c1805b76e3b3c37d385f4bccbb4cf
push id6576
push userrsayre@mozilla.com
push dateSat, 18 Apr 2009 15:37:12 +0000
treeherderautoland@55d20693b9cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs487930
milestone1.9.2a1pre
bug 487930 - removal of JSSLOT_ARRAY_LOOKUP_HOLDER. r=mrbkap
js/src/jsapi.cpp
js/src/jsarray.cpp
js/src/jsarray.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3190,40 +3190,42 @@ JS_AliasProperty(JSContext *cx, JSObject
                                    sprop->attrs, sprop->flags | SPROP_IS_ALIAS,
                                    sprop->shortid)
               != NULL);
     }
     OBJ_DROP_PROPERTY(cx, obj, prop);
     return ok;
 }
 
-static jsval
-LookupResult(JSContext *cx, JSObject *obj, JSObject *obj2, JSProperty *prop)
-{
-    JSScopeProperty *sprop;
-    jsval rval;
-
+static JSBool
+LookupResult(JSContext *cx, JSObject *obj, JSObject *obj2, JSProperty *prop,
+             jsval *vp)
+{
     if (!prop) {
         /* XXX bad API: no way to tell "not defined" from "void value" */
-        return JSVAL_VOID;
+        *vp = JSVAL_VOID;
+        return JS_TRUE;
     }
+
+    JSBool ok = JS_TRUE;
     if (OBJ_IS_NATIVE(obj2)) {
+        JSScopeProperty *sprop = (JSScopeProperty *) prop;
+
         /* Peek at the native property's slot value, without doing a Get. */
-        sprop = (JSScopeProperty *)prop;
-        rval = SPROP_HAS_VALID_SLOT(sprop, OBJ_SCOPE(obj2))
+        *vp = SPROP_HAS_VALID_SLOT(sprop, OBJ_SCOPE(obj2))
                ? LOCKED_OBJ_GET_SLOT(obj2, sprop->slot)
                : JSVAL_TRUE;
     } else if (OBJ_IS_DENSE_ARRAY(cx, obj2)) {
-        rval = js_GetDenseArrayElementValue(obj2, prop);
+        ok = js_GetDenseArrayElementValue(cx, obj2, prop, vp);
     } else {
         /* XXX bad API: no way to return "defined but value unknown" */
-        rval = JSVAL_TRUE;
+        *vp = JSVAL_TRUE;
     }
     OBJ_DROP_PROPERTY(cx, obj2, prop);
-    return rval;
+    return ok;
 }
 
 static JSBool
 GetPropertyAttributesById(JSContext *cx, JSObject *obj, jsid id, uintN flags,
                           JSBool own, JSPropertyDescriptor *desc)
 {
     JSObject *obj2;
     JSProperty *prop;
@@ -3456,39 +3458,33 @@ JS_HasPropertyById(JSContext *cx, JSObje
            OBJ_DROP_PROPERTY(cx, obj2, prop);
     }
     return ok;
 }
 
 JS_PUBLIC_API(JSBool)
 JS_LookupProperty(JSContext *cx, JSObject *obj, const char *name, jsval *vp)
 {
-    JSBool ok;
     JSObject *obj2;
     JSProperty *prop;
 
     CHECK_REQUEST(cx);
-    ok = LookupProperty(cx, obj, name, JSRESOLVE_QUALIFIED, &obj2, &prop);
-    if (ok)
-        *vp = LookupResult(cx, obj, obj2, prop);
-    return ok;
+    return LookupProperty(cx, obj, name, JSRESOLVE_QUALIFIED, &obj2, &prop) &&
+           LookupResult(cx, obj, obj2, prop, vp);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_LookupPropertyById(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
 {
-    JSBool ok;
     JSObject *obj2;
     JSProperty *prop;
 
     CHECK_REQUEST(cx);
-    ok = LookupPropertyById(cx, obj, id, JSRESOLVE_QUALIFIED, &obj2, &prop);
-    if (ok)
-        *vp = LookupResult(cx, obj, obj2, prop);
-    return ok;
+    return LookupPropertyById(cx, obj, id, JSRESOLVE_QUALIFIED, &obj2, &prop) &&
+           LookupResult(cx, obj, obj2, prop, vp);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_LookupPropertyWithFlags(JSContext *cx, JSObject *obj, const char *name,
                            uintN flags, jsval *vp)
 {
     JSAtom *atom;
     JSObject *obj2;
@@ -3506,17 +3502,17 @@ JS_LookupPropertyWithFlagsById(JSContext
     JSBool ok;
     JSProperty *prop;
 
     CHECK_REQUEST(cx);
     ok = OBJ_IS_NATIVE(obj)
          ? js_LookupPropertyWithFlags(cx, obj, id, flags, objp, &prop) >= 0
          : OBJ_LOOKUP_PROPERTY(cx, obj, id, objp, &prop);
     if (ok)
-        *vp = LookupResult(cx, obj, *objp, prop);
+        ok = LookupResult(cx, obj, *objp, prop, vp);
     return ok;
 }
 
 JS_PUBLIC_API(JSBool)
 JS_GetPropertyDescriptorById(JSContext *cx, JSObject *obj, jsid id, uintN flags,
                              JSPropertyDescriptor *desc)
 {
     CHECK_REQUEST(cx);
@@ -3726,26 +3722,23 @@ JS_HasUCProperty(JSContext *cx, JSObject
     return ok;
 }
 
 JS_PUBLIC_API(JSBool)
 JS_LookupUCProperty(JSContext *cx, JSObject *obj,
                     const jschar *name, size_t namelen,
                     jsval *vp)
 {
-    JSBool ok;
     JSObject *obj2;
     JSProperty *prop;
 
     CHECK_REQUEST(cx);
-    ok = LookupUCProperty(cx, obj, name, namelen, JSRESOLVE_QUALIFIED,
-                          &obj2, &prop);
-    if (ok)
-        *vp = LookupResult(cx, obj, obj2, prop);
-    return ok;
+    return LookupUCProperty(cx, obj, name, namelen, JSRESOLVE_QUALIFIED,
+                            &obj2, &prop) &&
+           LookupResult(cx, obj, obj2, prop, vp);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_GetUCProperty(JSContext *cx, JSObject *obj,
                  const jschar *name, size_t namelen,
                  jsval *vp)
 {
     JSAtom *atom;
@@ -3894,26 +3887,23 @@ JS_HasElement(JSContext *cx, JSObject *o
             OBJ_DROP_PROPERTY(cx, obj2, prop);
     }
     return ok;
 }
 
 JS_PUBLIC_API(JSBool)
 JS_LookupElement(JSContext *cx, JSObject *obj, jsint index, jsval *vp)
 {
-    JSBool ok;
     JSObject *obj2;
     JSProperty *prop;
 
     CHECK_REQUEST(cx);
-    ok = LookupPropertyById(cx, obj, INT_TO_JSID(index), JSRESOLVE_QUALIFIED,
-                            &obj2, &prop);
-    if (ok)
-        *vp = LookupResult(cx, obj, obj2, prop);
-    return ok;
+    return LookupPropertyById(cx, obj, INT_TO_JSID(index), JSRESOLVE_QUALIFIED,
+                              &obj2, &prop) &&
+           LookupResult(cx, obj, obj2, prop, vp);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_GetElement(JSContext *cx, JSObject *obj, jsint index, jsval *vp)
 {
     JSAutoResolveFlags rf(cx, JSRESOLVE_QUALIFIED);
 
     CHECK_REQUEST(cx);
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -48,18 +48,17 @@
  *
  * We track these pieces of metadata for arrays in dense mode:
  *  - the array's length property as a uint32, in JSSLOT_ARRAY_LENGTH,
  *  - the number of indices that are filled (non-holes), in JSSLOT_ARRAY_COUNT,
  *  - the net number of slots starting at dslots (capacity), in dslots[-1] if
  *    dslots is non-NULL.
  *
  * In dense mode, holes in the array are represented by JSVAL_HOLE.  The final
- * slot in fslots (JSSLOT_ARRAY_LOOKUP_HOLDER) is used to store the single jsid
- * "in use" by a lookupProperty caller.
+ * slot in fslots is unused.
  *
  * Arrays are converted to use js_SlowArrayClass when any of these conditions
  * are met:
  *  - the load factor (COUNT / capacity) is less than 0.25, and there are
  *    more than MIN_SPARSE_INDEX slots total
  *  - a property is set that is not indexed (and not "length"); or
  *  - a property is defined that has non-default property attributes.
  *
@@ -682,82 +681,75 @@ array_length_setter(JSContext *cx, JSObj
         if (!ok)
             return JS_FALSE;
     }
 
     obj->fslots[JSSLOT_ARRAY_LENGTH] = newlen;
     return JS_TRUE;
 }
 
+/*
+ * We have only indexed properties up to capacity (excepting holes), plus the
+ * length property. For all else, we delegate to the prototype.
+ */
+static inline bool
+IsDenseArrayId(JSContext *cx, JSObject *obj, jsid id)
+{
+    JS_ASSERT(OBJ_IS_DENSE_ARRAY(cx, obj));
+
+    uint32 i;
+    return id == ATOM_TO_JSID(cx->runtime->atomState.lengthAtom) ||
+           (js_IdIsIndex(id, &i) &&
+            obj->fslots[JSSLOT_ARRAY_LENGTH] != 0 &&
+            i < js_DenseArrayCapacity(obj) &&
+            obj->dslots[i] != JSVAL_HOLE);
+}
+
 static JSBool
 array_lookupProperty(JSContext *cx, JSObject *obj, jsid id, JSObject **objp,
                      JSProperty **propp)
 {
-    uint32 i;
-    union { JSProperty *p; jsval *v; } u;
-
     if (!OBJ_IS_DENSE_ARRAY(cx, obj))
         return js_LookupProperty(cx, obj, id, objp, propp);
 
-    /*
-     * We have only indexed properties up to capacity (excepting holes), plus
-     * the length property. For all else, we delegate to the prototype.
-     */
-    if (id != ATOM_TO_JSID(cx->runtime->atomState.lengthAtom) &&
-        (!js_IdIsIndex(id, &i) ||
-         obj->fslots[JSSLOT_ARRAY_LENGTH] == 0 ||
-         i >= js_DenseArrayCapacity(obj) ||
-         obj->dslots[i] == JSVAL_HOLE))
-    {
-        JSObject *proto = STOBJ_GET_PROTO(obj);
-
-        if (!proto) {
-            *objp = NULL;
-            *propp = NULL;
-            return JS_TRUE;
-        }
-
-        return OBJ_LOOKUP_PROPERTY(cx, proto, id, objp, propp);
+    if (IsDenseArrayId(cx, obj, id)) {
+        *propp = (JSProperty *) id;
+        *objp = obj;
+        return JS_TRUE;
     }
 
-    /* FIXME 417501: threadsafety: could race with a lookup on another thread.
-     * If we can only have a single lookup active per context, we could
-     * pigeonhole this on the context instead. */
-    JS_ASSERT(JSVAL_IS_VOID(obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER]));
-    obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER] = (jsval) id;
-    u.v = &(obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER]);
-    *propp = u.p;
-    *objp = obj;
-    return JS_TRUE;
+    JSObject *proto = STOBJ_GET_PROTO(obj);
+    if (!proto) {
+        *objp = NULL;
+        *propp = NULL;
+        return JS_TRUE;
+    }
+    return OBJ_LOOKUP_PROPERTY(cx, proto, id, objp, propp);
 }
 
 static void
 array_dropProperty(JSContext *cx, JSObject *obj, JSProperty *prop)
 {
-    JS_ASSERT_IF(OBJ_IS_DENSE_ARRAY(cx, obj),
-                 !JSVAL_IS_VOID(obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER]));
-#ifdef DEBUG
-    obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER] = JSVAL_VOID;
-#endif
+    JS_ASSERT(IsDenseArrayId(cx, obj, (jsid) prop));
 }
 
-jsval
-js_GetDenseArrayElementValue(JSObject *obj, JSProperty *prop)
+JSBool
+js_GetDenseArrayElementValue(JSContext *cx, JSObject *obj, JSProperty *prop,
+                             jsval *vp)
 {
-    /* OBJ_IS_DENSE_ARRAY does not use the cx argument. */
-    JS_ASSERT(OBJ_IS_DENSE_ARRAY(cx, obj));
-    JS_ASSERT((void *) prop ==
-              (void *) &(obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER]));
-    JS_ASSERT(JSVAL_IS_INT(obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER]));
-
-    jsint i = JSVAL_TO_INT(obj->fslots[JSSLOT_ARRAY_LOOKUP_HOLDER]);
-    JS_ASSERT(i >= 0);
-    jsval v = obj->dslots[i];
-    JS_ASSERT(v != JSVAL_HOLE);
-    return v;
+    jsid id = (jsid) prop;
+    JS_ASSERT(IsDenseArrayId(cx, obj, id));
+
+    uint32 i;
+    if (!js_IdIsIndex(id, &i)) {
+        JS_ASSERT(id == ATOM_TO_JSID(cx->runtime->atomState.lengthAtom));
+        return IndexToValue(cx, obj->fslots[JSSLOT_ARRAY_LENGTH], vp);
+    }
+    *vp = obj->dslots[i];
+    return JS_TRUE;
 }
 
 static JSBool
 array_getProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
 {
     uint32 i;
 
     if (id == ATOM_TO_JSID(cx->runtime->atomState.lengthAtom))
--- a/js/src/jsarray.h
+++ b/js/src/jsarray.h
@@ -99,17 +99,17 @@ js_NewArrayObject(JSContext *cx, jsuint 
 extern JSObject *
 js_NewSlowArrayObject(JSContext *cx);
 
 extern JSBool
 js_MakeArraySlow(JSContext *cx, JSObject *obj);
 
 #define JSSLOT_ARRAY_LENGTH            JSSLOT_PRIVATE
 #define JSSLOT_ARRAY_COUNT             (JSSLOT_ARRAY_LENGTH + 1)
-#define JSSLOT_ARRAY_LOOKUP_HOLDER     (JSSLOT_ARRAY_COUNT + 1)
+#define JSSLOT_ARRAY_UNUSED            (JSSLOT_ARRAY_COUNT + 1)
 
 static JS_INLINE uint32
 js_DenseArrayCapacity(JSObject *obj)
 {
     JS_ASSERT(OBJ_IS_DENSE_ARRAY(BOGUS_CX, obj));
     return obj->dslots ? (uint32) obj->dslots[-1] : 0;
 }
 
@@ -224,14 +224,15 @@ js_ArrayToJSDoubleBuffer(JSContext *cx, 
                          jsdouble *dest);
 
 JSBool
 js_PrototypeHasIndexedProperties(JSContext *cx, JSObject *obj);
 
 /*
  * Utility to access the value from the id returned by array_lookupProperty.
  */
-jsval
-js_GetDenseArrayElementValue(JSObject *obj, JSProperty *prop);
+JSBool
+js_GetDenseArrayElementValue(JSContext *cx, JSObject *obj, JSProperty *prop,
+                             jsval *vp);
 
 JS_END_EXTERN_C
 
 #endif /* jsarray_h___ */