From f8b5727e38a362443affe42d0c22da4e61d9bc15 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Tue, 27 Jul 2010 16:42:58 -0700 Subject: [PATCH] Bug 582081 - Dense array patches regressed empty Array creation on Dromaeo. r=jwalden --- js/src/jsarray.cpp | 61 ++++++++++++++++++++++--------------------- js/src/jsbuiltins.h | 2 +- js/src/jsobj.cpp | 7 +++-- js/src/jsobj.h | 4 ++- js/src/jsobjinlines.h | 8 +++--- js/src/jstracer.cpp | 41 ++++++++++++++--------------- 6 files changed, 63 insertions(+), 60 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 2f5cf72a4a05..2f907bca5394 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -321,7 +321,7 @@ JSObject::growDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap) 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) @@ -355,8 +355,7 @@ JSObject::ensureDenseArrayElements(JSContext *cx, uint32 newcap) */ 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) { /* @@ -1077,16 +1076,12 @@ JSObject::makeDenseArraySlow(JSContext *cx) 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), @@ -1459,8 +1454,9 @@ InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, const Value *vector 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)); @@ -2983,8 +2979,11 @@ js_Array(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value *rval) } 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); @@ -2995,28 +2994,33 @@ js_NewArrayWithSlots(JSContext* cx, JSObject* proto, uint32 len) obj->map = const_cast(&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; @@ -3038,11 +3042,8 @@ js_NewArrayObject(JSContext *cx, jsuint length, const Value *vector) */ 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; diff --git a/js/src/jsbuiltins.h b/js/src/jsbuiltins.h index 6eec44d7355b..aa76342d6ce2 100644 --- a/js/src/jsbuiltins.h +++ b/js/src/jsbuiltins.h @@ -577,7 +577,7 @@ 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. */ diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 99bf6627b0d6..8f66df811688 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -3421,9 +3421,12 @@ js_InitClass(JSContext *cx, JSObject *obj, JSObject *parent_proto, /* * 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 diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 82314b46d25a..05bef0a3df65 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -477,6 +477,8 @@ struct JSObject { // 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.) @@ -489,7 +491,7 @@ struct JSObject { 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); diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index a019b71727e6..e5fd335350f0 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -156,16 +156,15 @@ 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 & @@ -213,6 +212,7 @@ inline void JSObject::voidDenseOnlyArraySlots() { JS_ASSERT(isDenseArray()); + fslots[JSSLOT_DENSE_ARRAY_CAPACITY].setUndefined(); } inline void diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index c58d8d4607c4..32a6987139a7 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -10996,20 +10996,13 @@ TraceRecorder::newArray(JSObject* ctor, uint32 argc, Value* argv, Value* rval) 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 @@ -13649,11 +13642,9 @@ TraceRecorder::denseArrayElement(Value& oval, Value& ival, Value*& vp, LIns*& v_ * 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) { @@ -13672,6 +13663,8 @@ TraceRecorder::denseArrayElement(Value& oval, Value& ival, Value*& vp, LIns*& v_ 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, @@ -13975,13 +13968,17 @@ TraceRecorder::record_JSOP_NEWINIT() 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; @@ -15821,7 +15818,7 @@ TraceRecorder::record_JSOP_NEWARRAY() 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;