Bug 582081 - Dense array patches regressed empty Array creation on Dromaeo. r=jwalden
authorAndreas Gal <gal@mozilla.com>
Tue, 27 Jul 2010 16:42:58 -0700
changeset 48604 85ac7407fdf9f32f77e203ace4e11428d19e66e6
parent 48603 af1686bcc2632c49f9fdba973957a982a427a227
child 48605 4ffdb7de7b891937bd011086a92c54f40f2166d4
push id14748
push userrsayre@mozilla.com
push dateSun, 01 Aug 2010 00:33:23 +0000
treeherdermozilla-central@f0df797bb2a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs582081
milestone2.0b3pre
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 582081 - Dense array patches regressed empty Array creation on Dromaeo. r=jwalden
js/src/jsarray.cpp
js/src/jsbuiltins.h
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/jstracer.cpp
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -316,17 +316,17 @@ JSObject::growDenseArrayElements(JSConte
     JS_ASSERT(newcap >= oldcap);
 
     if (newcap > MAX_DSLOTS_LENGTH32) {
         if (!JS_ON_TRACE(cx))
             js_ReportAllocationOverflow(cx);
         return JS_FALSE;
     }
 
-    /* dslots can be briefly NULL during array creation */
+    /* dslots can be NULL during array creation. */
     Value *slots = dslots ? dslots - 1 : NULL;
     Value *newslots = (Value *) cx->realloc(slots, (size_t(newcap) + 1) * sizeof(Value));
     if (!newslots)
         return false;
 
     dslots = newslots + 1;
     setDenseArrayCapacity(newcap);
 
@@ -350,18 +350,17 @@ JSObject::ensureDenseArrayElements(JSCon
     static const size_t CAPACITY_DOUBLING_MAX = 1024 * 1024;
 
     /*
      * Round up all large allocations to a multiple of this (1MB), so as not
      * to waste space if malloc gives us 1MB-sized chunks (as jemalloc does).
      */
     static const size_t CAPACITY_CHUNK = 1024 * 1024 / sizeof(Value);
 
-    /* While creating arrays, dslots can be NULL. */
-    uint32 oldcap = dslots ? getDenseArrayCapacity() : 0;
+    uint32 oldcap = getDenseArrayCapacity();
 
     if (newcap > oldcap) {
         /*
          * If this overflows uint32, newcap is very large. nextsize will end
          * up being less than newcap, the code below will thus disregard it,
          * and resizeDenseArrayElements() will fail.
          *
          * The way we use dslots[-1] forces a few +1s and -1s here. For
@@ -1072,26 +1071,22 @@ JSObject::makeDenseArraySlow(JSContext *
         /* arrayProto is Array.prototype. */
         JS_ASSERT(arrayProto->getClass() == &js_SlowArrayClass);
         emptyShape = arrayProto->scope()->emptyScope->shape;
     }
     JSScope *scope = JSScope::create(cx, &js_ObjectOps, &js_SlowArrayClass, obj, emptyShape);
     if (!scope)
         return JS_FALSE;
 
+    uint32 capacity = obj->getDenseArrayCapacity();
+
     /* For a brief moment the object has NULL dslots until we slowify it during construction. */
-    uint32 capacity = dslots ? obj->getDenseArrayCapacity() : 0;
-    if (capacity) {
-        scope->freeslot = obj->numSlots() + JS_INITIAL_NSLOTS;
-        // XXX: changing the capacity like this is awful.  Bug 558263 will remove
-        // the need for this.
-        obj->setDenseArrayCapacity(JS_INITIAL_NSLOTS + capacity);
-    } else {
-        scope->freeslot = obj->numSlots();
-    }
+    if (obj->dslots)
+        obj->dslots[-1].setPrivateUint32(JS_INITIAL_NSLOTS + capacity);
+    scope->freeslot = obj->numSlots();
 
     /* Begin with the length property to share more of the property tree. */
     if (!scope->addProperty(cx, ATOM_TO_JSID(cx->runtime->atomState.lengthAtom),
                             array_length_getter, array_length_setter,
                             JSSLOT_ARRAY_LENGTH, JSPROP_PERMANENT | JSPROP_SHARED, 0, 0)) {
         goto out_bad;
     }
 
@@ -1454,18 +1449,19 @@ InitArrayElements(JSContext *cx, JSObjec
 
 static JSBool
 InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, const Value *vector)
 {
     JS_ASSERT(obj->isArray());
 
     JS_ASSERT(obj->isDenseArray());
     obj->setArrayLength(length);
+    obj->setDenseArrayCapacity(0);
     if (!vector || !length)
-        return obj->ensureDenseArrayElements(cx, ARRAY_CAPACITY_MIN);
+        return true;
     if (!obj->ensureDenseArrayElements(cx, length))
         return false;
     memcpy(obj->getDenseArrayElements(), vector, length * sizeof(Value));
     return true;
 }
 
 /*
  * Perl-inspired join, reverse, and sort.
@@ -2978,50 +2974,58 @@ js_Array(JSContext *cx, JSObject *obj, u
         if (argv[0].isNull())
             return JS_FALSE;
         vector = NULL;
     }
     return InitArrayObject(cx, obj, length, vector);
 }
 
 JSObject* JS_FASTCALL
-js_NewArrayWithSlots(JSContext* cx, JSObject* proto, uint32 len)
+js_NewEmptyArray(JSContext* cx, JSObject* proto, int32 len)
 {
+    if (len < 0)
+        return NULL;
+
     JS_ASSERT(proto->isArray());
 
     JSObject* obj = js_NewGCObject(cx);
     if (!obj)
         return NULL;
 
     /* Initialize all fields of JSObject. */
     obj->map = const_cast<JSObjectMap *>(&SharedArrayMap);
     obj->init(&js_ArrayClass, proto, proto->getParent(), NullValue());
     obj->setArrayLength(len);
+    obj->setDenseArrayCapacity(0);
+    return obj;
+}
+#ifdef JS_TRACER
+JS_DEFINE_CALLINFO_3(extern, OBJECT, js_NewEmptyArray, CONTEXT, OBJECT, INT32, 0,
+                     nanojit::ACC_STORE_ANY)
+#endif
+
+JSObject* JS_FASTCALL
+js_NewPreallocatedArray(JSContext* cx, JSObject* proto, int32 len)
+{
+    JSObject *obj = js_NewEmptyArray(cx, proto, len);
+    if (!obj)
+        return NULL;
     if (!obj->growDenseArrayElements(cx, 0, JS_MAX(len, ARRAY_CAPACITY_MIN)))
         return NULL;
     return obj;
 }
 #ifdef JS_TRACER
-JS_DEFINE_CALLINFO_3(extern, OBJECT, js_NewArrayWithSlots, CONTEXT, OBJECT, UINT32, 0,
+JS_DEFINE_CALLINFO_3(extern, OBJECT, js_NewPreallocatedArray, CONTEXT, OBJECT, INT32, 0,
                      nanojit::ACC_STORE_ANY)
 #endif
 
-JSObject* JS_FASTCALL
-js_NewEmptyArray(JSContext* cx, JSObject* proto)
-{
-    return js_NewArrayWithSlots(cx, proto, 0);
-}
-#ifdef JS_TRACER
-JS_DEFINE_CALLINFO_2(extern, OBJECT, js_NewEmptyArray, CONTEXT, OBJECT, 0, nanojit::ACC_STORE_ANY)
-#endif
-
 JSObject *
 js_InitArrayClass(JSContext *cx, JSObject *obj)
 {
-    JSObject *proto = js_InitClass(cx, obj, NULL, &js_ArrayClass, js_Array, 1,
+    JSObject *proto = js_InitClass(cx, obj, NULL, &js_SlowArrayClass, js_Array, 1,
                                    NULL, array_methods, NULL, array_static_methods);
     if (!proto)
         return NULL;
     proto->setArrayLength(0);
 
     return proto;
 }
 
@@ -3033,21 +3037,18 @@ js_NewArrayObject(JSContext *cx, jsuint 
         return NULL;
 
     /*
      * If this fails, the global object was not initialized and its class does
      * not have JSCLASS_IS_GLOBAL.
      */
     JS_ASSERT(obj->getProto());
 
-    {
-        AutoObjectRooter tvr(cx, obj);
-        if (!InitArrayObject(cx, obj, length, vector))
-            obj = NULL;
-    }
+    if (!InitArrayObject(cx, obj, length, vector))
+        obj = NULL;
 
     /* Set/clear newborn root, in case we lost it.  */
     cx->weakRoots.finalizableNewborns[FINALIZE_OBJECT] = obj;
     return obj;
 }
 
 JSObject *
 js_NewSlowArrayObject(JSContext *cx)
--- a/js/src/jsbuiltins.h
+++ b/js/src/jsbuiltins.h
@@ -572,17 +572,17 @@ js_dmod(jsdouble a, jsdouble b);
 
 #endif /* !JS_TRACER */
 
 /* Defined in jsarray.cpp. */
 JS_DECLARE_CALLINFO(js_Array_dense_setelem)
 JS_DECLARE_CALLINFO(js_Array_dense_setelem_int)
 JS_DECLARE_CALLINFO(js_Array_dense_setelem_double)
 JS_DECLARE_CALLINFO(js_NewEmptyArray)
-JS_DECLARE_CALLINFO(js_NewArrayWithSlots)
+JS_DECLARE_CALLINFO(js_NewPreallocatedArray)
 JS_DECLARE_CALLINFO(js_ArrayCompPush_tn)
 
 /* Defined in jsbuiltins.cpp. */
 JS_DECLARE_CALLINFO(js_UnboxDouble)
 JS_DECLARE_CALLINFO(js_UnboxInt32)
 JS_DECLARE_CALLINFO(js_dmod)
 JS_DECLARE_CALLINFO(js_imod)
 JS_DECLARE_CALLINFO(js_DoubleToInt32)
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3416,19 +3416,22 @@ js_InitClass(JSContext *cx, JSObject *ob
 
         AutoValueRooter tvr2(cx, ObjectValue(*fun));
         if (!DefineStandardSlot(cx, obj, key, atom, tvr2.value(), 0, named))
             goto bad;
 
         /*
          * Remember the class this function is a constructor for so that
          * we know to create an object of this class when we call the
-         * constructor.
+         * constructor. Arrays are a special case. We have a slow and a
+         * dense array. While the prototype is a slow array (it has
+         * named properties), we want to make dense arrays with the
+         * constructor, so we have to monkey patch that here.
          */
-        FUN_CLASP(fun) = clasp;
+        FUN_CLASP(fun) = (clasp == &js_SlowArrayClass) ? &js_ArrayClass : clasp;
 
         /*
          * Optionally construct the prototype object, before the class has
          * been fully initialized.  Allow the ctor to replace proto with a
          * different object, as is done for operator new -- and as at least
          * XML support requires.
          */
         ctor = FUN_OBJECT(fun);
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -472,29 +472,31 @@ struct JSObject {
     /*
      * Array-specific getters and setters (for both dense and slow arrays).
      */
 
   private:
     // Used by dense and slow arrays.
     static const uint32 JSSLOT_ARRAY_LENGTH = JSSLOT_PRIVATE;
 
+    static const uint32 JSSLOT_DENSE_ARRAY_CAPACITY = JSSLOT_PRIVATE + 1;
+
     // This assertion must remain true;  see comment in js_MakeArraySlow().
     // (Nb: This method is never called, it just contains a static assertion.
     // The static assertion isn't inline because that doesn't work on Mac.)
     inline void staticAssertArrayLengthIsInPrivateSlot();
 
   public:
     static const uint32 DENSE_ARRAY_FIXED_RESERVED_SLOTS = 3;
 
     inline uint32 getArrayLength() const;
     inline void setArrayLength(uint32 length);
 
     inline uint32 getDenseArrayCapacity() const;
-    inline void setDenseArrayCapacity(uint32 capacity); // XXX: bug 558263 will remove this
+    inline void setDenseArrayCapacity(uint32 capacity);
 
     inline const js::Value &getDenseArrayElement(uint32 i) const;
     inline js::Value *addressOfDenseArrayElement(uint32 i);
     inline void setDenseArrayElement(uint32 i, const js::Value &v);
 
     inline js::Value *getDenseArrayElements() const;   // returns pointer to the Array's elements array
     bool growDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap);
     bool ensureDenseArrayElements(JSContext *cx, uint32 newcap);
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -151,26 +151,25 @@ JSObject::setArrayLength(uint32 length)
     JS_ASSERT(isArray());
     fslots[JSSLOT_ARRAY_LENGTH].setPrivateUint32(length);
 }
 
 inline uint32
 JSObject::getDenseArrayCapacity() const
 {
     JS_ASSERT(isDenseArray());
-    JS_ASSERT(dslots);
-    return dslots[-1].toPrivateUint32();
+    return fslots[JSSLOT_DENSE_ARRAY_CAPACITY].toPrivateUint32();
 }
 
 inline void
 JSObject::setDenseArrayCapacity(uint32 capacity)
 {
     JS_ASSERT(isDenseArray());
-    JS_ASSERT(dslots);
-    dslots[-1].setPrivateUint32(capacity);
+    JS_ASSERT(!capacity == !dslots);
+    fslots[JSSLOT_DENSE_ARRAY_CAPACITY].setPrivateUint32(capacity);
 }
 
 inline const js::Value &
 JSObject::getDenseArrayElement(uint32 i) const
 {
     JS_ASSERT(isDenseArray());
     JS_ASSERT(i < getDenseArrayCapacity());
     return dslots[i];
@@ -208,16 +207,17 @@ JSObject::freeDenseArrayElements(JSConte
         dslots = NULL;
     }
 }
 
 inline void 
 JSObject::voidDenseOnlyArraySlots()
 {
     JS_ASSERT(isDenseArray());
+    fslots[JSSLOT_DENSE_ARRAY_CAPACITY].setUndefined();
 }
 
 inline void
 JSObject::setArgsLength(uint32 argc)
 {
     JS_ASSERT(isArguments());
     JS_ASSERT(argc <= JS_ARGS_LENGTH_MAX);
     fslots[JSSLOT_ARGS_LENGTH].setInt32(argc << 1);
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -10991,30 +10991,23 @@ TraceRecorder::newString(JSObject* ctor,
 
 RecordingStatus
 TraceRecorder::newArray(JSObject* ctor, uint32 argc, Value* argv, Value* rval)
 {
     LIns *proto_ins;
     CHECK_STATUS(getClassPrototype(ctor, proto_ins));
 
     LIns *arr_ins;
-    if (argc == 0) {
-        // arr_ins = js_NewEmptyArray(cx, Array.prototype)
-        LIns *args[] = { proto_ins, cx_ins };
+    if (argc == 0 || (argc == 1 && argv[0].isNumber())) {
+        LIns *args[] = { argc == 0 ? lir->insImmI(0) : d2i(get(argv)), proto_ins, cx_ins };
         arr_ins = lir->insCall(&js_NewEmptyArray_ci, args);
         guard(false, lir->insEqP_0(arr_ins), OOM_EXIT);
-    } else if (argc == 1 && argv[0].isNumber()) {
-        // arr_ins = js_NewEmptyArray(cx, Array.prototype, length)
-        LIns *args[] = { d2i(get(argv)), proto_ins, cx_ins }; // FIXME: is this 64-bit safe?
-        arr_ins = lir->insCall(&js_NewArrayWithSlots_ci, args);
-        guard(false, lir->insEqP_0(arr_ins), OOM_EXIT);
     } else {
-        // arr_ins = js_NewArrayWithSlots(cx, Array.prototype, argc)
         LIns *args[] = { INS_CONST(argc), proto_ins, cx_ins };
-        arr_ins = lir->insCall(&js_NewArrayWithSlots_ci, args);
+        arr_ins = lir->insCall(&js_NewPreallocatedArray_ci, args);
         guard(false, lir->insEqP_0(arr_ins), OOM_EXIT);
 
         // arr->dslots[i] = box_jsval(vp[i]);  for i in 0..argc
         LIns *dslots_ins = NULL;
         for (uint32 i = 0; i < argc && !outOfMemory(); i++) {
             stobj_set_dslot(arr_ins, i, dslots_ins, argv[i], get(&argv[i]));
         }
     }
@@ -13644,21 +13637,19 @@ TraceRecorder::denseArrayElement(Value& 
 
     /*
      * Arrays have both a length and a capacity, but we only need to check
      * |index < capacity|;  in the case where |length < index < capacity|
      * the entries [length..capacity-1] will have already been marked as
      * holes by resizeDenseArrayElements() so we can read them and get
      * the correct value.
      */
-    LIns* dslots_ins =
-        addName(lir->insLoad(LIR_ldp, obj_ins, offsetof(JSObject, dslots), ACC_OTHER), "dslots");
     LIns* capacity_ins =
-        addName(lir->insLoad(LIR_ldi, dslots_ins, -int(sizeof(jsval)), ACC_OTHER), "capacity");
-
+        addName(stobj_get_fslot_uint32(obj_ins, JSObject::JSSLOT_DENSE_ARRAY_CAPACITY),
+                "capacity");
     jsuint capacity = obj->getDenseArrayCapacity();
     bool within = (jsuint(idx) < capacity);
     if (!within) {
         /* If not idx < capacity, stay on trace (and read value as undefined). */
         guard(true, lir->ins2(LIR_geui, idx_ins, capacity_ins), exit);
 
         CHECK_STATUS(guardPrototypeHasNoIndexedProperties(obj, obj_ins, MISMATCH_EXIT));
 
@@ -13667,16 +13658,18 @@ TraceRecorder::denseArrayElement(Value& 
         addr_ins = NULL;
         return RECORD_CONTINUE;
     }
 
     /* Guard that index is within capacity. */
     guard(true, lir->ins2(LIR_ltui, idx_ins, capacity_ins), exit);
 
     /* Load the value and guard on its type to unbox it. */
+    LIns* dslots_ins =
+        addName(lir->insLoad(LIR_ldp, obj_ins, offsetof(JSObject, dslots), ACC_OTHER), "dslots");
     vp = &obj->dslots[jsuint(idx)];
 	JS_ASSERT(sizeof(Value) == 8); // The |3| in the following statement requires this.
     addr_ins = lir->ins2(LIR_addp, dslots_ins,
                          lir->ins2ImmI(LIR_lshp, lir->insUI2P(idx_ins), 3));
     v_ins = unbox_value(*vp, addr_ins, 0, exit, true);
 
     /* Don't let the hole value escape. Turn it into an undefined. */
     if (vp->isMagic()) {
@@ -13970,23 +13963,27 @@ TraceRecorder::record_JSOP_UINT16()
 
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_NEWINIT()
 {
     JSProtoKey key = JSProtoKey(GET_INT8(cx->regs->pc));
     LIns* proto_ins;
     CHECK_STATUS_A(getClassPrototype(key, proto_ins));
 
-    LIns* args[] = { proto_ins, cx_ins };
-    const CallInfo *ci = (key == JSProto_Array)
-                         ? &js_NewEmptyArray_ci
-                         : (cx->regs->pc[JSOP_NEWINIT_LENGTH] != JSOP_ENDINIT)
-                         ? &js_NonEmptyObject_ci
-                         : &js_Object_tn_ci;
-    LIns* v_ins = lir->insCall(ci, args);
+    LIns *v_ins;
+    if (key == JSProto_Array) {
+        LIns *args[] = { lir->insImmI(0), proto_ins, cx_ins };
+        v_ins = lir->insCall(&js_NewEmptyArray_ci, args);
+    } else {
+        LIns *args[] = { proto_ins, cx_ins };
+        v_ins = lir->insCall((cx->regs->pc[JSOP_NEWINIT_LENGTH] != JSOP_ENDINIT)
+                             ? &js_NonEmptyObject_ci
+                             : &js_Object_tn_ci,
+                             args);
+    }
     guard(false, lir->insEqP_0(v_ins), OOM_EXIT);
     stack(0, v_ins);
     return ARECORD_CONTINUE;
 }
 
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_ENDINIT()
 {
@@ -15816,17 +15813,17 @@ TraceRecorder::record_JSOP_NEWARRAY()
 {
     LIns *proto_ins;
     CHECK_STATUS_A(getClassPrototype(JSProto_Array, proto_ins));
 
     uint32 len = GET_UINT16(cx->regs->pc);
     cx->assertValidStackDepth(len);
 
     LIns* args[] = { lir->insImmI(len), proto_ins, cx_ins };
-    LIns* v_ins = lir->insCall(&js_NewArrayWithSlots_ci, args);
+    LIns* v_ins = lir->insCall(&js_NewPreallocatedArray_ci, args);
     guard(false, lir->insEqP_0(v_ins), OOM_EXIT);
 
     LIns* dslots_ins = NULL;
     uint32 count = 0;
     for (uint32 i = 0; i < len; i++) {
         Value& v = stackval(int(i) - int(len));
         if (!v.isMagic())
             count++;