Bug 807458 - Eliminate a SkipRoot from NewDenseCopiedArray. r=terrence
authorSteve Fink <sfink@mozilla.com>
Thu, 01 Nov 2012 13:57:47 -0700
changeset 112345 8fd8e9243788363cd60db34a050ff97069d78f66
parent 112344 13a34f4d0d67d4acdc5f9ce52d80592412411d85
child 112346 be5f1f5c13c3f003b967b73696b76e944a2daa36
push id23812
push useremorley@mozilla.com
push dateTue, 06 Nov 2012 14:01:34 +0000
treeherdermozilla-central@f4aeed115e54 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs807458
milestone19.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 807458 - Eliminate a SkipRoot from NewDenseCopiedArray. r=terrence
js/src/jsapi.cpp
js/src/jsarray.cpp
js/src/jsarray.h
js/src/jsstr.cpp
js/src/vm/ScopeObject.cpp
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4695,16 +4695,18 @@ JS_PUBLIC_API(void)
 JS_SetReservedSlot(RawObject obj, uint32_t index, RawValue value)
 {
     obj->setReservedSlot(index, value);
 }
 
 JS_PUBLIC_API(JSObject *)
 JS_NewArrayObject(JSContext *cx, int length, jsval *vector)
 {
+    AutoArrayRooter tvr(cx, length, vector);
+
     JS_THREADSAFE_ASSERT(cx->compartment != cx->runtime->atomsCompartment);
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
 
     assertSameCompartment(cx, JSValueArray(vector, vector ? (uint32_t)length : 0));
     return NewDenseCopiedArray(cx, (uint32_t)length, vector);
 }
 
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2598,18 +2598,17 @@ array_splice(JSContext *cx, unsigned arg
         actualDeleteCount = len - actualStart;
     }
 
     JS_ASSERT(len - actualStart >= actualDeleteCount);
 
     /* Steps 2, 8-9. */
     RootedObject arr(cx);
     if (CanOptimizeForDenseStorage(obj, actualStart, actualDeleteCount, cx)) {
-        arr = NewDenseCopiedArray(cx, actualDeleteCount,
-                                  obj->getDenseArrayElements() + actualStart);
+        arr = NewDenseCopiedArray(cx, actualDeleteCount, obj, actualStart);
         if (!arr)
             return false;
         TryReuseArrayType(obj, arr);
     } else {
         arr = NewDenseAllocatedArray(cx, actualDeleteCount);
         if (!arr)
             return false;
         TryReuseArrayType(obj, arr);
@@ -2798,19 +2797,18 @@ js::array_concat(JSContext *cx, unsigned
     RootedObject aobj(cx, ToObject(cx, args.thisv()));
     if (!aobj)
         return false;
 
     RootedObject nobj(cx);
     uint32_t length;
     if (aobj->isDenseArray()) {
         length = aobj->getArrayLength();
-        const Value *vector = aobj->getDenseArrayElements();
         uint32_t initlen = aobj->getDenseArrayInitializedLength();
-        nobj = NewDenseCopiedArray(cx, initlen, vector);
+        nobj = NewDenseCopiedArray(cx, initlen, aobj, 0);
         if (!nobj)
             return JS_FALSE;
         TryReuseArrayType(aobj, nobj);
         JSObject::setArrayLength(cx, nobj, length);
         args.rval().setObject(*nobj);
         if (argc == 0)
             return JS_TRUE;
         argc--;
@@ -2906,17 +2904,17 @@ array_slice(JSContext *cx, unsigned argc
 
     if (begin > end)
         begin = end;
 
     RootedObject nobj(cx);
 
     if (obj->isDenseArray() && end <= obj->getDenseArrayInitializedLength() &&
         !js_PrototypeHasIndexedProperties(obj)) {
-        nobj = NewDenseCopiedArray(cx, end - begin, obj->getDenseArrayElements() + begin);
+        nobj = NewDenseCopiedArray(cx, end - begin, obj, begin);
         if (!nobj)
             return JS_FALSE;
         TryReuseArrayType(obj, nobj);
         args.rval().setObject(*nobj);
         return JS_TRUE;
     }
 
     nobj = NewDenseAllocatedArray(cx, end - begin);
@@ -3703,35 +3701,51 @@ mjit::stubs::NewDenseUnallocatedArray(VM
     if (!obj)
         THROWV(NULL);
 
     return obj;
 }
 #endif
 
 JSObject *
-NewDenseCopiedArray(JSContext *cx, uint32_t length, const Value *vp, RawObject proto /* = NULL */)
+NewDenseCopiedArray(JSContext *cx, uint32_t length, HandleObject src, uint32_t elementOffset, RawObject proto /* = NULL */)
 {
-    // XXX vp may be an internal pointer to an object's dense array elements.
-    SkipRoot skip(cx, &vp);
-
     JSObject* obj = NewArray<true>(cx, length, proto);
     if (!obj)
         return NULL;
 
     JS_ASSERT(obj->getDenseArrayCapacity() >= length);
 
+    const Value* vp = src->getDenseArrayElements() + elementOffset;
     obj->setDenseArrayInitializedLength(vp ? length : 0);
 
     if (vp)
         obj->initDenseArrayElements(0, vp, length);
 
     return obj;
 }
 
+// values must point at already-rooted Value objects
+JSObject *
+NewDenseCopiedArray(JSContext *cx, uint32_t length, const Value *values, RawObject proto /* = NULL */)
+{
+    JSObject* obj = NewArray<true>(cx, length, proto);
+    if (!obj)
+        return NULL;
+
+    JS_ASSERT(obj->getDenseArrayCapacity() >= length);
+
+    obj->setDenseArrayInitializedLength(values ? length : 0);
+
+    if (values)
+        obj->initDenseArrayElements(0, values, length);
+
+    return obj;
+}
+
 JSObject *
 NewSlowEmptyArray(JSContext *cx)
 {
     RootedObject obj(cx, NewBuiltinClassInstance(cx, &SlowArrayClass));
     if (!obj || !AddLengthProperty(cx, obj))
         return NULL;
 
     JSObject::setArrayLength(cx, obj, 0);
--- a/js/src/jsarray.h
+++ b/js/src/jsarray.h
@@ -58,19 +58,23 @@ NewDenseAllocatedArray(JSContext *cx, ui
 
 /*
  * Create a dense array with a set length, but without allocating space for the
  * contents. This is useful, e.g., when accepting length from the user.
  */
 extern JSObject * JS_FASTCALL
 NewDenseUnallocatedArray(JSContext *cx, uint32_t length, RawObject proto = NULL);
 
-/* Create a dense array with a copy of vp. */
+/* Create a dense array with a copy of the dense array elements in src. */
 extern JSObject *
-NewDenseCopiedArray(JSContext *cx, uint32_t length, const Value *vp, RawObject proto = NULL);
+NewDenseCopiedArray(JSContext *cx, uint32_t length, HandleObject src, uint32_t elementOffset, RawObject proto = NULL);
+
+/* Create a dense array from the given array values, which must be rooted */
+extern JSObject *
+NewDenseCopiedArray(JSContext *cx, uint32_t length, const Value *values, RawObject proto = NULL);
 
 /* Create a sparse array. */
 extern JSObject *
 NewSlowEmptyArray(JSContext *cx);
 
 extern JSBool
 GetLengthProperty(JSContext *cx, HandleObject obj, uint32_t *lengthp);
 
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -2752,18 +2752,18 @@ js::str_split(JSContext *cx, unsigned ar
             return false;
         aobj->setType(type);
         args.rval().setObject(*aobj);
         return true;
     }
 
     /* Step 10. */
     if (!sepDefined) {
-        Value v = StringValue(str);
-        JSObject *aobj = NewDenseCopiedArray(cx, 1, &v);
+        RootedValue v(cx, StringValue(str));
+        JSObject *aobj = NewDenseCopiedArray(cx, 1, v.address());
         if (!aobj)
             return false;
         aobj->setType(type);
         args.rval().setObject(*aobj);
         return true;
     }
     Rooted<JSStableString*> stableStr(cx, str->ensureStable(cx));
     if (!stableStr)
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -1705,17 +1705,17 @@ DebugScopes::onPopCall(StackFrame *fp, J
      * invariants since DebugScopeObject::maybeSnapshot can already be NULL.
      */
     if (debugScope) {
         /*
          * Copy all frame values into the snapshot, regardless of
          * aliasing. This unnecessarily includes aliased variables
          * but it simplifies later indexing logic.
          */
-        StackFrame::CopyVector vec;
+        AutoValueVector vec(cx);
         if (!fp->copyRawFrameSlots(&vec) || vec.length() == 0)
             return;
 
         /*
          * Copy in formals that are not aliased via the scope chain
          * but are aliased via the arguments object.
          */
         RootedScript script(cx, fp->script());
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -212,17 +212,17 @@ StackFrame::pcQuadratic(const ContextSta
     if (StackFrame *next = seg.computeNextFrame(this, maxDepth))
         return next->prevpc();
 
     /* If we hit the limit, just return the beginning of the script. */
     return regs.fp()->script()->code;
 }
 
 bool
-StackFrame::copyRawFrameSlots(CopyVector *vec)
+StackFrame::copyRawFrameSlots(AutoValueVector *vec)
 {
     if (!vec->resize(numFormalArgs() + script()->nfixed))
         return false;
     PodCopy(vec->begin(), formals(), numFormalArgs());
     PodCopy(vec->begin() + numFormalArgs(), slots(), script()->nfixed);
     return true;
 }
 
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -492,18 +492,17 @@ class StackFrame
     inline Value &unaliasedVar(unsigned i, MaybeCheckAliasing = CHECK_ALIASING);
     inline Value &unaliasedLocal(unsigned i, MaybeCheckAliasing = CHECK_ALIASING);
 
     bool hasArgs() const { return isNonEvalFunctionFrame(); }
     inline Value &unaliasedFormal(unsigned i, MaybeCheckAliasing = CHECK_ALIASING);
     inline Value &unaliasedActual(unsigned i, MaybeCheckAliasing = CHECK_ALIASING);
     template <class Op> inline void forEachUnaliasedActual(Op op);
 
-    typedef Vector<Value, 16, SystemAllocPolicy> CopyVector;
-    bool copyRawFrameSlots(CopyVector *v);
+    bool copyRawFrameSlots(AutoValueVector *v);
 
     inline unsigned numFormalArgs() const;
     inline unsigned numActualArgs() const;
 
     inline Value &canonicalActualArg(unsigned i) const;
     template <class Op>
     inline bool forEachCanonicalActualArg(Op op, unsigned start = 0, unsigned count = unsigned(-1));
     template <class Op> inline bool forEachFormalArg(Op op);