Bug 919544 - Allow cached object allocation to GC; r=jandem
authorTerrence Cole <terrence@mozilla.com>
Sat, 01 Feb 2014 12:04:03 -0800
changeset 170559 9cdf5a8140238aff4e9cca8822bca261b9151d82
parent 170558 6cf3762073dcf3f2c87b50d04afe4defd5814133
child 170560 ad35d51d978158ca52ad58ffa46d8965ae6e4673
push id40250
push usertcole@mozilla.com
push dateWed, 26 Feb 2014 00:27:01 +0000
treeherdermozilla-inbound@9cdf5a814023 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs919544
milestone30.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 919544 - Allow cached object allocation to GC; r=jandem
js/public/RootingAPI.h
js/src/jsarray.cpp
js/src/jsgcinlines.h
js/src/jsobj.cpp
js/src/vm/ArrayObject-inl.h
js/src/vm/ArrayObject.h
js/src/vm/Runtime-inl.h
js/src/vm/Runtime.h
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -1075,17 +1075,17 @@ class FakeMutableHandle : public js::Mut
     template <typename S>
     void operator=(S v) MOZ_DELETE;
 
     void operator=(const FakeMutableHandle<T>& other) MOZ_DELETE;
 };
 
 /*
  * Types for a variable that either should or shouldn't be rooted, depending on
- * the template parameter Rooted. Used for implementing functions that can
+ * the template parameter allowGC. Used for implementing functions that can
  * operate on either rooted or unrooted data.
  *
  * The toHandle() and toMutableHandle() functions are for calling functions
  * which require handle types and are only called in the CanGC case. These
  * allow the calling code to type check.
  */
 enum AllowGC {
     NoGC = 0,
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -677,17 +677,17 @@ js::ArraySetLength(typename ExecutionMod
 
     if (mode == ParallelExecution) {
         // Overflowing int32 requires changing TI state.
         if (newLen > INT32_MAX)
             return false;
         arr->setLengthInt32(newLen);
     } else {
         JSContext *cx = cxArg->asJSContext();
-        ArrayObject::setLength(cx, arr, newLen);
+        arr->setLength(cx, newLen);
     }
 
 
     // All operations past here until the |!succeeded| code must be infallible,
     // so that all element fields remain properly synchronized.
 
     // Trim the initialized length, if needed, to preserve the <= length
     // invariant.  (Capacity was already reduced during element deletion, if
@@ -782,17 +782,17 @@ array_addProperty(JSContext *cx, HandleO
     uint32_t index;
     if (!js_IdIsIndex(id, &index))
         return true;
 
     uint32_t length = arr->length();
     if (index >= length) {
         MOZ_ASSERT(arr->lengthIsWritable(),
                    "how'd this element get added if length is non-writable?");
-        ArrayObject::setLength(cx, arr, index + 1);
+        arr->setLength(cx, index + 1);
     }
     return true;
 }
 
 bool
 js::ObjectMayHaveExtraIndexedProperties(JSObject *obj)
 {
     /*
@@ -2605,17 +2605,17 @@ js::array_concat(JSContext *cx, unsigned
     uint32_t length;
     if (aobj->is<ArrayObject>() && !aobj->isIndexed()) {
         length = aobj->as<ArrayObject>().length();
         uint32_t initlen = aobj->getDenseInitializedLength();
         narr = NewDenseCopiedArray(cx, initlen, aobj, 0);
         if (!narr)
             return false;
         TryReuseArrayType(aobj, narr);
-        ArrayObject::setLength(cx, narr, length);
+        narr->setLength(cx, length);
         args.rval().setObject(*narr);
         if (argc == 0)
             return true;
         argc--;
         p++;
     } else {
         narr = NewDenseEmptyArray(cx);
         if (!narr)
@@ -3043,17 +3043,17 @@ js_Array(JSContext *cx, unsigned argc, V
     if (!obj)
         return false;
     Rooted<ArrayObject*> arr(cx, &obj->as<ArrayObject>());
 
     arr->setType(type);
 
     /* If the length calculation overflowed, make sure that is marked for the new type. */
     if (arr->length() > INT32_MAX)
-        ArrayObject::setLength(cx, arr, arr->length());
+        arr->setLength(cx, arr->length());
 
     args.rval().setObject(*arr);
     return true;
 }
 
 JSObject *
 js_InitArrayClass(JSContext *cx, HandleObject obj)
 {
@@ -3142,26 +3142,31 @@ NewArray(ExclusiveContext *cxArg, uint32
 
     NewObjectCache::EntryIndex entry = -1;
     if (JSContext *cx = cxArg->maybeJSContext()) {
         NewObjectCache &cache = cx->runtime()->newObjectCache;
         if (newKind == GenericObject &&
             !cx->compartment()->hasObjectMetadataCallback() &&
             cache.lookupGlobal(&ArrayObject::class_, cx->global(), allocKind, &entry))
         {
-            RootedObject obj(cx, cache.newObjectFromHit(cx, entry,
-                                                        GetInitialHeap(newKind, &ArrayObject::class_)));
+            gc::InitialHeap heap = GetInitialHeap(newKind, &ArrayObject::class_);
+            JSObject *obj = cache.newObjectFromHit<NoGC>(cx, entry, heap);
             if (obj) {
                 /* Fixup the elements pointer and length, which may be incorrect. */
-                Rooted<ArrayObject*> arr(cx, &obj->as<ArrayObject>());
+                ArrayObject *arr = &obj->as<ArrayObject>();
                 arr->setFixedElements();
-                ArrayObject::setLength(cx, arr, length);
+                arr->setLength(cx, length);
                 if (allocateCapacity && !EnsureNewArrayElements(cx, arr, length))
                     return nullptr;
                 return arr;
+            } else {
+                RootedObject proto(cxArg, protoArg);
+                obj = cache.newObjectFromHit<CanGC>(cx, entry, heap);
+                JS_ASSERT(!obj);
+                protoArg = proto;
             }
         }
     }
 
     RootedObject proto(cxArg, protoArg);
     if (protoArg)
         JS::PoisonPtr(&protoArg);
 
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -520,16 +520,53 @@ AllocateNonObject(ThreadSafeContext *cx)
     T *t = static_cast<T *>(cx->allocator()->arenas.allocateFromFreeList(kind, thingSize));
     if (!t)
         t = static_cast<T *>(js::gc::ArenaLists::refillFreeList<allowGC>(cx, kind));
 
     CheckIncrementalZoneState(cx, t);
     return t;
 }
 
+/*
+ * When allocating for initialization from a cached object copy, we will
+ * potentially destroy the cache entry we want to copy if we allow GC. On the
+ * other hand, since these allocations are extremely common, we don't want to
+ * delay GC from these allocation sites. Instead we allow the GC, but still
+ * fail the allocation, forcing the non-cached path.
+ */
+template <AllowGC allowGC>
+inline JSObject *
+AllocateObjectForCacheHit(JSContext *cx, AllocKind kind, InitialHeap heap)
+{
+#ifdef JSGC_GENERATIONAL
+    if (ShouldNurseryAllocate(cx->nursery(), kind, heap)) {
+        size_t thingSize = Arena::thingSize(kind);
+
+        JS_ASSERT(thingSize == Arena::thingSize(kind));
+        if (!CheckAllocatorState<NoGC>(cx, kind))
+            return nullptr;
+
+        JSObject *obj = TryNewNurseryObject<NoGC>(cx, thingSize, 0);
+        if (!obj && allowGC) {
+            MinorGC(cx, JS::gcreason::OUT_OF_NURSERY);
+            return nullptr;
+        }
+        return obj;
+    }
+#endif
+
+    JSObject *obj = AllocateObject<NoGC>(cx, kind, 0, heap);
+    if (!obj && allowGC) {
+        MaybeGC(cx);
+        return nullptr;
+    }
+
+    return obj;
+}
+
 } /* namespace gc */
 
 template <js::AllowGC allowGC>
 inline JSObject *
 NewGCObject(js::ThreadSafeContext *cx, js::gc::AllocKind kind, size_t nDynamicSlots, js::gc::InitialHeap heap)
 {
     JS_ASSERT(kind >= js::gc::FINALIZE_OBJECT0 && kind <= js::gc::FINALIZE_OBJECT_LAST);
     return js::gc::AllocateObject<allowGC>(cx, kind, nDynamicSlots, heap);
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1305,42 +1305,50 @@ NewObjectCache::fillProto(EntryIndex ent
 {
     JS_ASSERT_IF(proto.isObject(), !proto.toObject()->is<GlobalObject>());
     JS_ASSERT(obj->getTaggedProto() == proto);
     return fill(entry, clasp, proto.raw(), kind, obj);
 }
 
 JSObject *
 js::NewObjectWithGivenProto(ExclusiveContext *cxArg, const js::Class *clasp,
-                            js::TaggedProto proto_, JSObject *parent_,
+                            js::TaggedProto protoArg, JSObject *parentArg,
                             gc::AllocKind allocKind, NewObjectKind newKind)
 {
-    Rooted<TaggedProto> proto(cxArg, proto_);
-    RootedObject parent(cxArg, parent_);
-
     if (CanBeFinalizedInBackground(allocKind, clasp))
         allocKind = GetBackgroundAllocKind(allocKind);
 
     NewObjectCache::EntryIndex entry = -1;
     if (JSContext *cx = cxArg->maybeJSContext()) {
         NewObjectCache &cache = cx->runtime()->newObjectCache;
-        if (proto.isObject() &&
+        if (protoArg.isObject() &&
             newKind == GenericObject &&
             !cx->compartment()->hasObjectMetadataCallback() &&
-            (!parent || parent == proto.toObject()->getParent()) &&
-            !proto.toObject()->is<GlobalObject>())
+            (!parentArg || parentArg == protoArg.toObject()->getParent()) &&
+            !protoArg.toObject()->is<GlobalObject>())
         {
-            if (cache.lookupProto(clasp, proto.toObject(), allocKind, &entry)) {
-                JSObject *obj = cache.newObjectFromHit(cx, entry, GetInitialHeap(newKind, clasp));
-                if (obj)
+            if (cache.lookupProto(clasp, protoArg.toObject(), allocKind, &entry)) {
+                JSObject *obj = cache.newObjectFromHit<NoGC>(cx, entry, GetInitialHeap(newKind, clasp));
+                if (obj) {
                     return obj;
+                } else {
+                    Rooted<TaggedProto> proto(cxArg, protoArg);
+                    RootedObject parent(cxArg, parentArg);
+                    obj = cache.newObjectFromHit<CanGC>(cx, entry, GetInitialHeap(newKind, clasp));
+                    JS_ASSERT(!obj);
+                    parentArg = parent;
+                    protoArg = proto;
+                }
             }
         }
     }
 
+    Rooted<TaggedProto> proto(cxArg, protoArg);
+    RootedObject parent(cxArg, parentArg);
+
     types::TypeObject *type = cxArg->getNewType(clasp, proto, nullptr);
     if (!type)
         return nullptr;
 
     /*
      * Default parent to the parent of the prototype, which was set from
      * the parent of the prototype's constructor.
      */
@@ -1388,19 +1396,27 @@ js::NewObjectWithClassProtoCommon(Exclus
     if (JSContext *cx = cxArg->maybeJSContext()) {
         NewObjectCache &cache = cx->runtime()->newObjectCache;
         if (parentArg->is<GlobalObject>() &&
             protoKey != JSProto_Null &&
             newKind == GenericObject &&
             !cx->compartment()->hasObjectMetadataCallback())
         {
             if (cache.lookupGlobal(clasp, &parentArg->as<GlobalObject>(), allocKind, &entry)) {
-                JSObject *obj = cache.newObjectFromHit(cx, entry, GetInitialHeap(newKind, clasp));
-                if (obj)
+                JSObject *obj = cache.newObjectFromHit<NoGC>(cx, entry, GetInitialHeap(newKind, clasp));
+                if (obj) {
                     return obj;
+                } else {
+                    RootedObject parent(cxArg, parentArg);
+                    RootedObject proto(cxArg, protoArg);
+                    obj = cache.newObjectFromHit<CanGC>(cx, entry, GetInitialHeap(newKind, clasp));
+                    JS_ASSERT(!obj);
+                    protoArg = proto;
+                    parentArg = parent;
+                }
             }
         }
     }
 
     RootedObject parent(cxArg, parentArg);
     RootedObject proto(cxArg, protoArg);
 
     if (!FindProto(cxArg, clasp, &proto))
@@ -1440,19 +1456,23 @@ js::NewObjectWithType(JSContext *cx, Han
     NewObjectCache &cache = cx->runtime()->newObjectCache;
 
     NewObjectCache::EntryIndex entry = -1;
     if (parent == type->proto().toObject()->getParent() &&
         newKind == GenericObject &&
         !cx->compartment()->hasObjectMetadataCallback())
     {
         if (cache.lookupType(type, allocKind, &entry)) {
-            JSObject *obj = cache.newObjectFromHit(cx, entry, GetInitialHeap(newKind, type->clasp()));
-            if (obj)
+            JSObject *obj = cache.newObjectFromHit<NoGC>(cx, entry, GetInitialHeap(newKind, type->clasp()));
+            if (obj) {
                 return obj;
+            } else {
+                obj = cache.newObjectFromHit<CanGC>(cx, entry, GetInitialHeap(newKind, type->clasp()));
+                parent = type->proto().toObject()->getParent();
+            }
         }
     }
 
     JSObject *obj = NewObject(cx, type, parent, allocKind, newKind);
     if (!obj)
         return nullptr;
 
     if (entry != -1 && !obj->hasDynamicSlots())
@@ -3596,26 +3616,26 @@ CallAddPropertyHook(typename ExecutionMo
 template <ExecutionMode mode>
 static inline bool
 CallAddPropertyHookDense(typename ExecutionModeTraits<mode>::ExclusiveContextType cxArg,
                          const Class *clasp, HandleObject obj, uint32_t index,
                          HandleValue nominal)
 {
     /* Inline addProperty for array objects. */
     if (obj->is<ArrayObject>()) {
-        Rooted<ArrayObject*> arr(cxArg, &obj->as<ArrayObject>());
+        ArrayObject *arr = &obj->as<ArrayObject>();
         uint32_t length = arr->length();
         if (index >= length) {
             if (mode == ParallelExecution) {
                 /* We cannot deal with overflows in parallel. */
                 if (length > INT32_MAX)
                     return false;
                 arr->setLengthInt32(index + 1);
             } else {
-                ArrayObject::setLength(cxArg->asExclusiveContext(), arr, index + 1);
+                arr->setLength(cxArg->asExclusiveContext(), index + 1);
             }
         }
         return true;
     }
 
     if (clasp->addProperty != JS_PropertyStub) {
         if (mode == ParallelExecution)
             return false;
--- a/js/src/vm/ArrayObject-inl.h
+++ b/js/src/vm/ArrayObject-inl.h
@@ -10,25 +10,25 @@
 #include "vm/ArrayObject.h"
 
 #include "vm/String.h"
 
 #include "jsinferinlines.h"
 
 namespace js {
 
-/* static */ inline void
-ArrayObject::setLength(ExclusiveContext *cx, Handle<ArrayObject*> arr, uint32_t length)
+inline void
+ArrayObject::setLength(ExclusiveContext *cx, uint32_t length)
 {
-    JS_ASSERT(arr->lengthIsWritable());
+    JS_ASSERT(lengthIsWritable());
 
     if (length > INT32_MAX) {
         /* Track objects with overflowing lengths in type information. */
-        types::MarkTypeObjectFlags(cx, arr, types::OBJECT_FLAG_LENGTH_OVERFLOW);
+        types::MarkTypeObjectFlags(cx, this, types::OBJECT_FLAG_LENGTH_OVERFLOW);
     }
 
-    arr->getElementsHeader()->length = length;
+    getElementsHeader()->length = length;
 }
 
 } // namespace js
 
 #endif // vm_ArrayObject_inl_h
 
--- a/js/src/vm/ArrayObject.h
+++ b/js/src/vm/ArrayObject.h
@@ -22,17 +22,17 @@ class ArrayObject : public JSObject
     bool lengthIsWritable() const {
         return !getElementsHeader()->hasNonwritableArrayLength();
     }
 
     uint32_t length() const {
         return getElementsHeader()->length;
     }
 
-    static inline void setLength(ExclusiveContext *cx, Handle<ArrayObject*> arr, uint32_t length);
+    inline void setLength(ExclusiveContext *cx, uint32_t length);
 
     // Variant of setLength for use on arrays where the length cannot overflow int32_t.
     void setLengthInt32(uint32_t length) {
         JS_ASSERT(lengthIsWritable());
         JS_ASSERT(length <= INT32_MAX);
         getElementsHeader()->length = length;
     }
 };
--- a/js/src/vm/Runtime-inl.h
+++ b/js/src/vm/Runtime-inl.h
@@ -32,16 +32,17 @@ NewObjectCache::lookupGlobal(const Class
 
 inline void
 NewObjectCache::fillGlobal(EntryIndex entry, const Class *clasp, js::GlobalObject *global, gc::AllocKind kind, JSObject *obj)
 {
     //JS_ASSERT(global == obj->getGlobal());
     return fill(entry, clasp, global, kind, obj);
 }
 
+template <AllowGC allowGC>
 inline JSObject *
 NewObjectCache::newObjectFromHit(JSContext *cx, EntryIndex entry_, js::gc::InitialHeap heap)
 {
     // The new object cache does not account for metadata attached via callbacks.
     JS_ASSERT(!cx->compartment()->hasObjectMetadataCallback());
 
     JS_ASSERT(unsigned(entry_) < mozilla::ArrayLength(entries));
     Entry *entry = &entries[entry_];
@@ -53,17 +54,26 @@ NewObjectCache::newObjectFromHit(JSConte
     types::TypeObject *type = templateObj->type_;
 
     if (type->shouldPreTenure())
         heap = gc::TenuredHeap;
 
     if (cx->runtime()->upcomingZealousGC())
         return nullptr;
 
-    JSObject *obj = js::NewGCObject<NoGC>(cx, entry->kind, 0, heap);
+    // Trigger an identical allocation to the one that notified us of OOM
+    // so that we trigger the right kind of GC automatically.
+    if (allowGC) {
+        JSObject *obj = js::gc::AllocateObjectForCacheHit<allowGC>(cx, entry->kind, heap);
+        JS_ASSERT(!obj);
+        return nullptr;
+    }
+
+    JS_ASSERT(allowGC == NoGC);
+    JSObject *obj = js::gc::AllocateObjectForCacheHit<NoGC>(cx, entry->kind, heap);
     if (obj) {
         copyCachedToObject(obj, templateObj, entry->kind);
         probes::CreateObject(cx, obj);
         return obj;
     }
 
     return nullptr;
 }
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -347,16 +347,17 @@ class NewObjectCache
         return lookup(type->clasp(), type, kind, pentry);
     }
 
     /*
      * Return a new object from a cache hit produced by a lookup method, or
      * nullptr if returning the object could possibly trigger GC (does not
      * indicate failure).
      */
+    template <AllowGC allowGC>
     inline JSObject *newObjectFromHit(JSContext *cx, EntryIndex entry, js::gc::InitialHeap heap);
 
     /* Fill an entry after a cache miss. */
     void fillProto(EntryIndex entry, const Class *clasp, js::TaggedProto proto, gc::AllocKind kind, JSObject *obj);
 
     inline void fillGlobal(EntryIndex entry, const Class *clasp, js::GlobalObject *global,
                            gc::AllocKind kind, JSObject *obj);