Bug 1258453 - Compact arenas containing strings r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 23 Mar 2016 13:27:25 +0000
changeset 290111 36c1fd35d9959fa380d07521b210ba315772d683
parent 290110 56d3ab31480a27c4e6063f635f78625b8ce4d220
child 290112 66fd82b72d1e2b584e50b3200dff2f08f4ed88c0
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1258453
milestone48.0a1
Bug 1258453 - Compact arenas containing strings r=terrence
js/src/gc/GCInternals.h
js/src/jsfriendapi.h
js/src/jsgc.cpp
js/src/jsgc.h
js/src/jsiter.cpp
js/src/jsiter.h
js/src/vm/String.cpp
js/src/vm/String.h
--- a/js/src/gc/GCInternals.h
+++ b/js/src/gc/GCInternals.h
@@ -120,16 +120,17 @@ CheckHashTablesAfterMovingGC(JSRuntime* 
 #endif
 
 struct MovingTracer : JS::CallbackTracer
 {
     explicit MovingTracer(JSRuntime* rt) : CallbackTracer(rt, TraceWeakMapKeysValues) {}
 
     void onObjectEdge(JSObject** objp) override;
     void onShapeEdge(Shape** shapep) override;
+    void onStringEdge(JSString** stringp) override;
     void onChild(const JS::GCCellPtr& thing) override {
         MOZ_ASSERT(!RelocationOverlay::isCellForwarded(thing.asCell()));
     }
 
 #ifdef DEBUG
     TracerKind getTracerKind() const override { return TracerKind::Moving; }
 #endif
 };
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -1369,16 +1369,21 @@ class MOZ_STACK_CLASS AutoStableStringCh
         state_ = Uninitialized;
         ownsChars_ = false;
         return true;
     }
 
   private:
     AutoStableStringChars(const AutoStableStringChars& other) = delete;
     void operator=(const AutoStableStringChars& other) = delete;
+
+    bool baseIsInline(JS::Handle<JSLinearString*> linearString);
+    bool copyLatin1Chars(JSContext*, JS::Handle<JSLinearString*> linearString);
+    bool copyTwoByteChars(JSContext*, JS::Handle<JSLinearString*> linearString);
+    bool copyAndInflateLatin1Chars(JSContext*, JS::Handle<JSLinearString*> linearString);
 };
 
 struct MOZ_STACK_CLASS JS_FRIEND_API(ErrorReport)
 {
     explicit ErrorReport(JSContext* cx);
     ~ErrorReport();
 
     bool init(JSContext* cx, JS::HandleValue exn);
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2035,17 +2035,20 @@ static const AllocKind AllocKindsToReloc
     AllocKind::OBJECT4_BACKGROUND,
     AllocKind::OBJECT8,
     AllocKind::OBJECT8_BACKGROUND,
     AllocKind::OBJECT12,
     AllocKind::OBJECT12_BACKGROUND,
     AllocKind::OBJECT16,
     AllocKind::OBJECT16_BACKGROUND,
     AllocKind::SHAPE,
-    AllocKind::ACCESSOR_SHAPE
+    AllocKind::ACCESSOR_SHAPE,
+    AllocKind::FAT_INLINE_STRING,
+    AllocKind::STRING,
+    AllocKind::EXTERNAL_STRING
 };
 
 Arena*
 ArenaList::removeRemainingArenas(Arena** arenap)
 {
     // This is only ever called to remove arenas that are after the cursor, so
     // we don't need to update it.
 #ifdef DEBUG
@@ -2383,16 +2386,24 @@ void
 MovingTracer::onShapeEdge(Shape** shapep)
 {
     Shape* shape = *shapep;
     if (IsForwarded(shape))
         *shapep = Forwarded(shape);
 }
 
 void
+MovingTracer::onStringEdge(JSString** stringp)
+{
+    JSString* string = *stringp;
+    if (IsForwarded(string))
+        *stringp = Forwarded(string);
+}
+
+void
 Zone::prepareForCompacting()
 {
     FreeOp* fop = runtimeFromMainThread()->defaultFreeOp();
     discardJitCode(fop);
 }
 
 void
 GCRuntime::sweepTypesAfterCompacting(Zone* zone)
@@ -2448,17 +2459,17 @@ UpdateCellPointersTyped(MovingTracer* tr
     for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) {
         T* cell = reinterpret_cast<T*>(i.getCell());
         cell->fixupAfterMovingGC();
         TraceChildren(trc, cell, traceKind);
     }
 }
 
 /*
- * Update the interal pointers for all cells in an arena.
+ * Update the internal pointers for all cells in an arena.
  */
 static void
 UpdateCellPointers(MovingTracer* trc, Arena* arena)
 {
     AllocKind kind = arena->getAllocKind();
     JS::TraceKind traceKind = MapAllocToTraceKind(kind);
 
     switch (kind) {
@@ -2491,16 +2502,19 @@ UpdateCellPointers(MovingTracer* trc, Ar
         UpdateCellPointersTyped<AccessorShape>(trc, arena, traceKind);
         return;
       case AllocKind::BASE_SHAPE:
         UpdateCellPointersTyped<BaseShape>(trc, arena, traceKind);
         return;
       case AllocKind::OBJECT_GROUP:
         UpdateCellPointersTyped<ObjectGroup>(trc, arena, traceKind);
         return;
+      case AllocKind::STRING:
+        UpdateCellPointersTyped<JSString>(trc, arena, traceKind);
+        return;
       case AllocKind::JITCODE:
         UpdateCellPointersTyped<jit::JitCode>(trc, arena, traceKind);
         return;
       default:
         MOZ_CRASH("Invalid alloc kind for UpdateCellPointers");
     }
 }
 
@@ -2537,19 +2551,20 @@ struct ArenasToUpdate
     Arena* next(AutoLockHelperThreadState& lock);
 };
 
 ArenasToUpdate::UpdateKind
 ArenasToUpdate::updateKind(AllocKind kind)
 {
     MOZ_ASSERT(IsValidAllocKind(kind));
 
-    // GC things that do not contain JSObject pointers don't need updating.
+    // GC things that do not contain pointers to cells we relocate don't need
+    // updating. Note that AllocKind::STRING is the only string kind which can
+    // be a rope and hence contain relocatable pointers.
     if (kind == AllocKind::FAT_INLINE_STRING ||
-        kind == AllocKind::STRING ||
         kind == AllocKind::EXTERNAL_STRING ||
         kind == AllocKind::SYMBOL)
     {
         return NONE;
     }
 
     // We try to update as many GC things in parallel as we can, but there are
     // kinds for which this might not be safe:
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1102,17 +1102,18 @@ template <typename T>
 struct MightBeForwarded
 {
     static_assert(mozilla::IsBaseOf<Cell, T>::value,
                   "T must derive from Cell");
     static_assert(!mozilla::IsSame<Cell, T>::value && !mozilla::IsSame<TenuredCell, T>::value,
                   "T must not be Cell or TenuredCell");
 
     static const bool value = mozilla::IsBaseOf<JSObject, T>::value ||
-                              mozilla::IsBaseOf<Shape, T>::value;
+                              mozilla::IsBaseOf<Shape, T>::value ||
+                              mozilla::IsBaseOf<JSString, T>::value;
 };
 
 template <typename T>
 inline bool
 IsForwarded(T* t)
 {
     RelocationOverlay* overlay = RelocationOverlay::fromCell(t);
     if (!MightBeForwarded<T>::value) {
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -48,20 +48,20 @@ using mozilla::Maybe;
 using mozilla::PodCopy;
 using mozilla::PodZero;
 
 typedef Rooted<PropertyIteratorObject*> RootedPropertyIteratorObject;
 
 static const gc::AllocKind ITERATOR_FINALIZE_KIND = gc::AllocKind::OBJECT2_BACKGROUND;
 
 void
-NativeIterator::mark(JSTracer* trc)
+NativeIterator::trace(JSTracer* trc)
 {
     for (HeapPtrFlatString* str = begin(); str < end(); str++)
-        TraceEdge(trc, str, "prop");
+        TraceNullableEdge(trc, str, "prop");
     TraceNullableEdge(trc, &obj, "obj");
 
     for (size_t i = 0; i < guard_length; i++)
         guard_array[i].trace(trc);
 
     // The SuppressDeletedPropertyHelper loop can GC, so make sure that if the
     // GC removes any elements from the list, it won't remove this one.
     if (iterObj_)
@@ -580,40 +580,32 @@ NewPropertyIteratorObject(JSContext* cx,
         if (!DefineProperty(cx, res, cx->names().next, value))
             return nullptr;
     }
 
     return res;
 }
 
 NativeIterator*
-NativeIterator::allocateIterator(JSContext* cx, uint32_t numGuards, const AutoIdVector& props)
+NativeIterator::allocateIterator(JSContext* cx, uint32_t numGuards, uint32_t plength)
 {
     JS_STATIC_ASSERT(sizeof(ReceiverGuard) == 2 * sizeof(void*));
 
-    size_t plength = props.length();
-    NativeIterator* ni = cx->zone()->pod_malloc_with_extra<NativeIterator, void*>(plength + numGuards * 2);
+    size_t extraLength = plength + numGuards * 2;
+    NativeIterator* ni = cx->zone()->pod_malloc_with_extra<NativeIterator, void*>(extraLength);
     if (!ni) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
-    AutoValueVector strings(cx);
-    ni->props_array = ni->props_cursor = reinterpret_cast<HeapPtrFlatString*>(ni + 1);
+    void** extra = reinterpret_cast<void**>(ni + 1);
+    PodZero(ni);
+    PodZero(extra, extraLength);
+    ni->props_array = ni->props_cursor = reinterpret_cast<HeapPtrFlatString*>(extra);
     ni->props_end = ni->props_array + plength;
-    if (plength) {
-        for (size_t i = 0; i < plength; i++) {
-            JSFlatString* str = IdToString(cx, props[i]);
-            if (!str || !strings.append(StringValue(str)))
-                return nullptr;
-            ni->props_array[i].init(str);
-        }
-    }
-    ni->next_ = nullptr;
-    ni->prev_ = nullptr;
     return ni;
 }
 
 NativeIterator*
 NativeIterator::allocateSentinel(JSContext* maybecx)
 {
     NativeIterator* ni = js_pod_malloc<NativeIterator>();
     if (!ni) {
@@ -635,16 +627,37 @@ NativeIterator::init(JSObject* obj, JSOb
     this->obj.init(obj);
     this->iterObj_ = iterObj;
     this->flags = flags;
     this->guard_array = (HeapReceiverGuard*) this->props_end;
     this->guard_length = numGuards;
     this->guard_key = key;
 }
 
+bool
+NativeIterator::initProperties(JSContext* cx, Handle<PropertyIteratorObject*> obj,
+                               const AutoIdVector& props)
+{
+    // The obj parameter is just so that we can ensure that this object will get
+    // traced if we GC.
+    MOZ_ASSERT(this == obj->getNativeIterator());
+
+    size_t plength = props.length();
+    MOZ_ASSERT(plength == size_t(end() - begin()));
+
+    for (size_t i = 0; i < plength; i++) {
+        JSFlatString* str = IdToString(cx, props[i]);
+        if (!str)
+            return false;
+        props_array[i].init(str);
+    }
+
+    return true;
+}
+
 static inline void
 RegisterEnumerator(JSContext* cx, PropertyIteratorObject* iterobj, NativeIterator* ni)
 {
     /* Register non-escaping native enumerators (for-in) with the current context. */
     if (ni->flags & JSITER_ENUMERATE) {
         ni->link(cx->compartment()->enumerators);
 
         MOZ_ASSERT(!(ni->flags & JSITER_ACTIVE));
@@ -661,33 +674,36 @@ VectorToKeyIterator(JSContext* cx, Handl
     if (obj->isSingleton() && !obj->setIteratedSingleton(cx))
         return false;
     MarkObjectGroupFlags(cx, obj, OBJECT_FLAG_ITERATED);
 
     Rooted<PropertyIteratorObject*> iterobj(cx, NewPropertyIteratorObject(cx, flags));
     if (!iterobj)
         return false;
 
-    NativeIterator* ni = NativeIterator::allocateIterator(cx, numGuards, keys);
+    NativeIterator* ni = NativeIterator::allocateIterator(cx, numGuards, keys.length());
     if (!ni)
         return false;
+
+    iterobj->setNativeIterator(ni);
     ni->init(obj, iterobj, flags, numGuards, key);
+    if (!ni->initProperties(cx, iterobj, keys))
+        return false;
 
     if (numGuards) {
         // Fill in the guard array from scratch.
         JSObject* pobj = obj;
         size_t ind = 0;
         do {
             ni->guard_array[ind++].init(ReceiverGuard(pobj));
             pobj = pobj->getProto();
         } while (pobj);
         MOZ_ASSERT(ind == numGuards);
     }
 
-    iterobj->setNativeIterator(ni);
     objp.set(iterobj);
 
     RegisterEnumerator(cx, iterobj, ni);
     return true;
 }
 
 static bool
 VectorToValueIterator(JSContext* cx, HandleObject obj, unsigned flags, AutoIdVector& keys,
@@ -698,22 +714,25 @@ VectorToValueIterator(JSContext* cx, Han
     if (obj->isSingleton() && !obj->setIteratedSingleton(cx))
         return false;
     MarkObjectGroupFlags(cx, obj, OBJECT_FLAG_ITERATED);
 
     Rooted<PropertyIteratorObject*> iterobj(cx, NewPropertyIteratorObject(cx, flags));
     if (!iterobj)
         return false;
 
-    NativeIterator* ni = NativeIterator::allocateIterator(cx, 0, keys);
+    NativeIterator* ni = NativeIterator::allocateIterator(cx, 0, keys.length());
     if (!ni)
         return false;
-    ni->init(obj, iterobj, flags, 0, 0);
 
     iterobj->setNativeIterator(ni);
+    ni->init(obj, iterobj, flags, 0, 0);
+    if (!ni->initProperties(cx, iterobj, keys))
+        return false;
+
     objp.set(iterobj);
 
     RegisterEnumerator(cx, iterobj, ni);
     return true;
 }
 
 bool
 js::EnumeratedIdVectorToIterator(JSContext* cx, HandleObject obj, unsigned flags,
@@ -729,22 +748,25 @@ js::EnumeratedIdVectorToIterator(JSConte
 bool
 js::NewEmptyPropertyIterator(JSContext* cx, unsigned flags, MutableHandleObject objp)
 {
     Rooted<PropertyIteratorObject*> iterobj(cx, NewPropertyIteratorObject(cx, flags));
     if (!iterobj)
         return false;
 
     AutoIdVector keys(cx); // Empty
-    NativeIterator* ni = NativeIterator::allocateIterator(cx, 0, keys);
+    NativeIterator* ni = NativeIterator::allocateIterator(cx, 0, keys.length());
     if (!ni)
         return false;
-    ni->init(nullptr, iterobj, flags, 0, 0);
 
     iterobj->setNativeIterator(ni);
+    ni->init(nullptr, iterobj, flags, 0, 0);
+    if (!ni->initProperties(cx, iterobj, keys))
+        return false;
+
     objp.set(iterobj);
 
     RegisterEnumerator(cx, iterobj, ni);
     return true;
 }
 
 static inline void
 UpdateNativeIterator(NativeIterator* ni, JSObject* obj)
@@ -1047,17 +1069,17 @@ PropertyIteratorObject::sizeOfMisc(mozil
 {
     return mallocSizeOf(getPrivate());
 }
 
 void
 PropertyIteratorObject::trace(JSTracer* trc, JSObject* obj)
 {
     if (NativeIterator* ni = obj->as<PropertyIteratorObject>().getNativeIterator())
-        ni->mark(trc);
+        ni->trace(trc);
 }
 
 void
 PropertyIteratorObject::finalize(FreeOp* fop, JSObject* obj)
 {
     if (NativeIterator* ni = obj->as<PropertyIteratorObject>().getNativeIterator())
         fop->free_(ni);
 }
@@ -1483,23 +1505,22 @@ js::InitLegacyIteratorClass(JSContext* c
     if (global->getPrototype(JSProto_Iterator).isObject())
         return &global->getPrototype(JSProto_Iterator).toObject();
 
     RootedObject iteratorProto(cx);
     iteratorProto = global->createBlankPrototype(cx, &PropertyIteratorObject::class_);
     if (!iteratorProto)
         return nullptr;
 
-    AutoIdVector blank(cx);
-    NativeIterator* ni = NativeIterator::allocateIterator(cx, 0, blank);
+    NativeIterator* ni = NativeIterator::allocateIterator(cx, 0, 0);
     if (!ni)
         return nullptr;
-    ni->init(nullptr, nullptr, 0 /* flags */, 0, 0);
 
     iteratorProto->as<PropertyIteratorObject>().setNativeIterator(ni);
+    ni->init(nullptr, nullptr, 0 /* flags */, 0, 0);
 
     Rooted<JSFunction*> ctor(cx);
     ctor = global->createConstructor(cx, IteratorConstructor, cx->names().Iterator, 2);
     if (!ctor)
         return nullptr;
     if (!LinkConstructorAndPrototype(cx, ctor, iteratorProto))
         return nullptr;
     if (!DefinePropertiesAndFunctions(cx, iteratorProto, nullptr, legacy_iterator_methods))
--- a/js/src/jsiter.h
+++ b/js/src/jsiter.h
@@ -23,16 +23,18 @@
  * For cacheable native iterators, whether the iterator is currently active.
  * Not serialized by XDR.
  */
 #define JSITER_ACTIVE       0x1000
 #define JSITER_UNREUSABLE   0x2000
 
 namespace js {
 
+class PropertyIteratorObject;
+
 struct NativeIterator
 {
     HeapPtrObject obj;                  // Object being iterated.
     JSObject* iterObj_;                 // Internal iterator object.
     HeapPtrFlatString* props_array;
     HeapPtrFlatString* props_cursor;
     HeapPtrFlatString* props_end;
     HeapReceiverGuard* guard_array;
@@ -99,21 +101,22 @@ struct NativeIterator
 
         next_->prev_ = prev_;
         prev_->next_ = next_;
         next_ = nullptr;
         prev_ = nullptr;
     }
 
     static NativeIterator* allocateSentinel(JSContext* maybecx);
-    static NativeIterator* allocateIterator(JSContext* cx, uint32_t slength,
-                                            const js::AutoIdVector& props);
+    static NativeIterator* allocateIterator(JSContext* cx, uint32_t slength, uint32_t plength);
     void init(JSObject* obj, JSObject* iterObj, unsigned flags, uint32_t slength, uint32_t key);
+    bool initProperties(JSContext* cx, Handle<PropertyIteratorObject*> obj,
+                        const js::AutoIdVector& props);
 
-    void mark(JSTracer* trc);
+    void trace(JSTracer* trc);
 
     static void destroy(NativeIterator* iter) {
         js_free(iter);
     }
 };
 
 class PropertyIteratorObject : public NativeObject
 {
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -889,16 +889,23 @@ bool
 AutoStableStringChars::init(JSContext* cx, JSString* s)
 {
     RootedLinearString linearString(cx, s->ensureLinear(cx));
     if (!linearString)
         return false;
 
     MOZ_ASSERT(state_ == Uninitialized);
 
+    // If the chars are inline then we need to copy them since they may be moved
+    // by a compacting GC.
+    if (baseIsInline(linearString)) {
+        return linearString->hasTwoByteChars() ? copyTwoByteChars(cx, linearString)
+                                               : copyLatin1Chars(cx, linearString);
+    }
+
     if (linearString->hasLatin1Chars()) {
         state_ = Latin1;
         latin1Chars_ = linearString->rawLatin1Chars();
     } else {
         state_ = TwoByte;
         twoByteChars_ = linearString->rawTwoByteChars();
     }
 
@@ -910,38 +917,92 @@ bool
 AutoStableStringChars::initTwoByte(JSContext* cx, JSString* s)
 {
     RootedLinearString linearString(cx, s->ensureLinear(cx));
     if (!linearString)
         return false;
 
     MOZ_ASSERT(state_ == Uninitialized);
 
-    if (linearString->hasTwoByteChars()) {
-        state_ = TwoByte;
-        twoByteChars_ = linearString->rawTwoByteChars();
-        s_ = linearString;
-        return true;
-    }
+    if (linearString->hasLatin1Chars())
+        return copyAndInflateLatin1Chars(cx, linearString);
+
+    // If the chars are inline then we need to copy them since they may be moved
+    // by a compacting GC.
+    if (baseIsInline(linearString))
+        return copyTwoByteChars(cx, linearString);
 
+    state_ = TwoByte;
+    twoByteChars_ = linearString->rawTwoByteChars();
+    s_ = linearString;
+    return true;
+}
+
+bool AutoStableStringChars::baseIsInline(HandleLinearString linearString)
+{
+    JSString* base = linearString;
+    while (base->isDependent())
+        base = base->asDependent().base();
+    return base->isInline();
+}
+
+bool
+AutoStableStringChars::copyAndInflateLatin1Chars(JSContext* cx, HandleLinearString linearString)
+{
     char16_t* chars = cx->pod_malloc<char16_t>(linearString->length() + 1);
     if (!chars)
         return false;
 
     CopyAndInflateChars(chars, linearString->rawLatin1Chars(),
                         linearString->length());
     chars[linearString->length()] = 0;
 
     state_ = TwoByte;
     ownsChars_ = true;
     twoByteChars_ = chars;
     s_ = linearString;
     return true;
 }
 
+bool
+AutoStableStringChars::copyLatin1Chars(JSContext* cx, HandleLinearString linearString)
+{
+    size_t length = linearString->length();
+    JS::Latin1Char* chars = cx->pod_malloc<JS::Latin1Char>(length + 1);
+    if (!chars)
+        return false;
+
+    PodCopy(chars, linearString->rawLatin1Chars(), length);
+    chars[length] = 0;
+
+    state_ = Latin1;
+    ownsChars_ = true;
+    latin1Chars_ = chars;
+    s_ = linearString;
+    return true;
+}
+
+bool
+AutoStableStringChars::copyTwoByteChars(JSContext* cx, HandleLinearString linearString)
+{
+    size_t length = linearString->length();
+    char16_t* chars = cx->pod_malloc<char16_t>(length + 1);
+    if (!chars)
+        return false;
+
+    PodCopy(chars, linearString->rawTwoByteChars(), length);
+    chars[length] = 0;
+
+    state_ = TwoByte;
+    ownsChars_ = true;
+    twoByteChars_ = chars;
+    s_ = linearString;
+    return true;
+}
+
 #ifdef DEBUG
 void
 JSAtom::dump()
 {
     fprintf(stderr, "JSAtom* (%p) = ", (void*) this);
     this->JSString::dump();
 }
 
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -467,16 +467,18 @@ class JSString : public js::gc::TenuredC
     inline JSLinearString* base() const;
 
     void traceBase(JSTracer* trc);
 
     /* Only called by the GC for strings with the AllocKind::STRING kind. */
 
     inline void finalize(js::FreeOp* fop);
 
+    void fixupAfterMovingGC() {}
+
     /* Gets the number of bytes that the chars take on the heap. */
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     /* Offsets for direct field from jit code. */
 
     static size_t offsetOfLength() {
         return offsetof(JSString, d.u1.length);