Bug 903519 - Strings in the nursery: tracing and tenuring, r=jonco
☠☠ backed out by 65e92478e09d ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Tue, 17 Oct 2017 12:35:06 -0700
changeset 450641 7c96258a64595d287eb72a54ee52a656dbb40365
parent 450640 11b3f0fda4adb74211c197ab157796fcafc27b95
child 450642 7d56db66836900bc7758c6829b9235a3dd26947e
push id8531
push userryanvm@gmail.com
push dateFri, 12 Jan 2018 16:47:01 +0000
treeherdermozilla-beta@0bc627ade5a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs903519
milestone59.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 903519 - Strings in the nursery: tracing and tenuring, r=jonco
js/src/gc/Marking.cpp
js/src/gc/Nursery-inl.h
js/src/gc/Nursery.cpp
js/src/gc/Nursery.h
js/src/jscompartment.cpp
js/src/vm/String-inl.h
js/src/vm/String.cpp
js/src/vm/String.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2689,30 +2689,43 @@ TenuringTracer::traverse(T** tp)
 
 template <>
 void
 TenuringTracer::traverse(JSObject** objp)
 {
     // We only ever visit the internals of objects after moving them to tenured.
     MOZ_ASSERT(!nursery().isInside(objp));
 
-    JSObject* obj = *objp;
-    if (!IsInsideNursery(obj) || nursery().getForwardedPointer(objp))
+    Cell** cellp = reinterpret_cast<Cell**>(objp);
+    if (!IsInsideNursery(*cellp) || nursery().getForwardedPointer(cellp))
         return;
 
     // Take a fast path for tenuring a plain object which is by far the most
     // common case.
+    JSObject* obj = *objp;
     if (obj->is<PlainObject>()) {
         *objp = movePlainObjectToTenured(&obj->as<PlainObject>());
         return;
     }
 
     *objp = moveToTenuredSlow(obj);
 }
 
+template <>
+void
+TenuringTracer::traverse(JSString** strp)
+{
+    // We only ever visit the internals of strings after moving them to tenured.
+    MOZ_ASSERT(!nursery().isInside(strp));
+
+    Cell** cellp = reinterpret_cast<Cell**>(strp);
+    if (IsInsideNursery(*cellp) && !nursery().getForwardedPointer(cellp))
+        *strp = moveToTenured(*strp);
+}
+
 template <typename S>
 struct TenuringTraversalFunctor : public IdentityDefaultAdaptor<S> {
     template <typename T> S operator()(T* t, TenuringTracer* trc) {
         trc->traverse(&t);
         return js::gc::RewrapTaggedPointer<S, T>::wrap(t);
     }
 };
 
@@ -2851,27 +2864,42 @@ js::gc::StoreBuffer::traceWholeCells(Ten
 
 void
 js::gc::StoreBuffer::CellPtrEdge::trace(TenuringTracer& mover) const
 {
     if (!*edge)
         return;
 
     MOZ_ASSERT(IsCellPointerValid(*edge));
-    MOZ_ASSERT((*edge)->getTraceKind() == JS::TraceKind::Object);
-    mover.traverse(reinterpret_cast<JSObject**>(edge));
+
+#ifdef DEBUG
+    auto traceKind = (*edge)->getTraceKind();
+    MOZ_ASSERT(traceKind == JS::TraceKind::Object || traceKind == JS::TraceKind::String);
+#endif
+
+    // Bug 1376646: Make separate store buffers for strings and objects, and
+    // only check IsInsideNursery once.
+
+    if (!IsInsideNursery(*edge))
+        return;
+
+    if (JSString::nurseryCellIsString(*edge))
+        mover.traverse(reinterpret_cast<JSString**>(edge));
+    else
+        mover.traverse(reinterpret_cast<JSObject**>(edge));
 }
 
 void
 js::gc::StoreBuffer::ValueEdge::trace(TenuringTracer& mover) const
 {
     if (deref())
         mover.traverse(edge);
 }
 
+
 struct TenuringFunctor
 {
     template <typename T>
     void operator()(T* thing, TenuringTracer& mover) {
         mover.traverse(thing);
     }
 };
 
@@ -2919,30 +2947,36 @@ js::TenuringTracer::traceSlots(Value* vp
 }
 
 inline void
 js::TenuringTracer::traceSlots(JS::Value* vp, uint32_t nslots)
 {
     traceSlots(vp, vp + nslots);
 }
 
+void
+js::TenuringTracer::traceString(JSString* str)
+{
+    str->traceChildren(this);
+}
+
 #ifdef DEBUG
 static inline ptrdiff_t
 OffsetToChunkEnd(void* p)
 {
     return ChunkLocationOffset - (uintptr_t(p) & gc::ChunkMask);
 }
 #endif
 
 /* Insert the given relocation entry into the list of things to visit. */
 inline void
-js::TenuringTracer::insertIntoFixupList(RelocationOverlay* entry) {
-    *tail = entry;
-    tail = &entry->nextRef();
-    *tail = nullptr;
+js::TenuringTracer::insertIntoObjectFixupList(RelocationOverlay* entry) {
+    *objTail = entry;
+    objTail = &entry->nextRef();
+    *objTail = nullptr;
 }
 
 template <typename T>
 inline T*
 js::TenuringTracer::allocTenured(Zone* zone, AllocKind kind) {
     TenuredCell* t = zone->arenas.allocateFromFreeList(kind, Arena::thingSize(kind));
     if (!t) {
         AutoEnterOOMUnsafeRegion oomUnsafe;
@@ -3019,17 +3053,17 @@ js::TenuringTracer::moveToTenuredSlow(JS
         tenuredSize += op(dst, src);
     } else {
         MOZ_ASSERT_IF(src->getClass()->hasFinalize(),
                       CanNurseryAllocateFinalizedClass(src->getClass()));
     }
 
     RelocationOverlay* overlay = RelocationOverlay::fromCell(src);
     overlay->forwardTo(dst);
-    insertIntoFixupList(overlay);
+    insertIntoObjectFixupList(overlay);
 
     TracePromoteToTenured(src, dst);
     return dst;
 }
 
 inline JSObject*
 js::TenuringTracer::movePlainObjectToTenured(PlainObject* src)
 {
@@ -3051,17 +3085,17 @@ js::TenuringTracer::movePlainObjectToTen
     // Move the slots and elements.
     tenuredSize += moveSlotsToTenured(dst, src, dstKind);
     tenuredSize += moveElementsToTenured(dst, src, dstKind);
 
     MOZ_ASSERT(!dst->getClass()->extObjectMovedOp());
 
     RelocationOverlay* overlay = RelocationOverlay::fromCell(src);
     overlay->forwardTo(dst);
-    insertIntoFixupList(overlay);
+    insertIntoObjectFixupList(overlay);
 
     TracePromoteToTenured(src, dst);
     return dst;
 }
 
 size_t
 js::TenuringTracer::moveSlotsToTenured(NativeObject* dst, NativeObject* src, AllocKind dstKind)
 {
@@ -3135,31 +3169,89 @@ js::TenuringTracer::moveElementsToTenure
 
     js_memcpy(dstHeader, srcAllocatedHeader, nslots * sizeof(HeapSlot));
     dst->elements_ = dstHeader->elements() + numShifted;
     nursery().setElementsForwardingPointer(srcHeader, dst->getElementsHeader(),
                                            srcHeader->capacity);
     return nslots * sizeof(HeapSlot);
 }
 
+inline void
+js::TenuringTracer::insertIntoStringFixupList(RelocationOverlay* entry) {
+    *stringTail = entry;
+    stringTail = &entry->nextRef();
+    *stringTail = nullptr;
+}
+
+JSString*
+js::TenuringTracer::moveToTenured(JSString* src)
+{
+    MOZ_ASSERT(IsInsideNursery(src));
+    MOZ_ASSERT(!src->zone()->usedByHelperThread());
+
+    AllocKind dstKind = src->getAllocKind();
+    Zone* zone = src->zone();
+
+    TenuredCell* t = zone->arenas.allocateFromFreeList(dstKind, Arena::thingSize(dstKind));
+    if (!t) {
+        AutoEnterOOMUnsafeRegion oomUnsafe;
+        t = runtime()->gc.refillFreeListInGC(zone, dstKind);
+        if (!t)
+            oomUnsafe.crash(ChunkSize, "Failed to allocate string while tenuring.");
+    }
+    JSString* dst = reinterpret_cast<JSString*>(t);
+    tenuredSize += moveStringToTenured(dst, src, dstKind);
+
+    RelocationOverlay* overlay = RelocationOverlay::fromCell(src);
+    overlay->forwardTo(dst);
+    insertIntoStringFixupList(overlay);
+
+    TracePromoteToTenured(src, dst);
+    return dst;
+}
+
 void
 js::Nursery::collectToFixedPoint(TenuringTracer& mover, TenureCountCache& tenureCounts)
 {
-    for (RelocationOverlay* p = mover.head; p; p = p->next()) {
+    for (RelocationOverlay* p = mover.objHead; p; p = p->next()) {
         JSObject* obj = static_cast<JSObject*>(p->forwardingAddress());
         mover.traceObject(obj);
 
         TenureCount& entry = tenureCounts.findEntry(obj->groupRaw());
         if (entry.group == obj->groupRaw()) {
             entry.count++;
         } else if (!entry.group) {
             entry.group = obj->groupRaw();
             entry.count = 1;
         }
     }
+
+    for (RelocationOverlay* p = mover.stringHead; p; p = p->next())
+        mover.traceString(static_cast<JSString*>(p->forwardingAddress()));
+}
+
+size_t
+js::TenuringTracer::moveStringToTenured(JSString* dst, JSString* src, AllocKind dstKind)
+{
+    size_t size = Arena::thingSize(dstKind);
+
+    // At the moment, strings always have the same AllocKind between src and
+    // dst. This may change in the future.
+    MOZ_ASSERT(dst->asTenured().getAllocKind() == src->getAllocKind());
+
+    // Copy the Cell contents.
+    MOZ_ASSERT(OffsetToChunkEnd(src) >= ptrdiff_t(size));
+    js_memcpy(dst, src, size);
+
+    if (src->isLinear() && !src->isInline() && !src->hasBase()) {
+        void* chars = src->asLinear().nonInlineCharsRaw();
+        nursery().removeMallocedBuffer(chars);
+    }
+
+    return size;
 }
 
 
 /*** IsMarked / IsAboutToBeFinalized **************************************************************/
 
 template <typename T>
 static inline void
 CheckIsMarkedThing(T* thingp)
@@ -3215,17 +3307,18 @@ template <>
 /* static */ bool
 IsMarkedInternal(JSRuntime* rt, JSObject** thingp)
 {
     if (IsOwnedByOtherRuntime(rt, *thingp))
         return true;
 
     if (IsInsideNursery(*thingp)) {
         MOZ_ASSERT(CurrentThreadCanAccessRuntime(rt));
-        return Nursery::getForwardedPointer(thingp);
+        Cell** cellp = reinterpret_cast<Cell**>(thingp);
+        return Nursery::getForwardedPointer(cellp);
     }
     return IsMarkedInternalCommon(thingp);
 }
 
 template <typename S>
 struct IsMarkedFunctor : public IdentityDefaultAdaptor<S> {
     template <typename T> S operator()(T* t, JSRuntime* rt, bool* rv) {
         *rv = IsMarkedInternal(rt, &t);
@@ -3261,17 +3354,17 @@ IsAboutToBeFinalizedInternal(T** thingp)
     JSRuntime* rt = thing->runtimeFromAnyThread();
 
     /* Permanent atoms are never finalized by non-owning runtimes. */
     if (ThingIsPermanentAtomOrWellKnownSymbol(thing) && TlsContext.get()->runtime() != rt)
         return false;
 
     if (IsInsideNursery(thing)) {
         return JS::CurrentThreadIsHeapMinorCollecting() &&
-               !Nursery::getForwardedPointer(reinterpret_cast<JSObject**>(thingp));
+               !Nursery::getForwardedPointer(reinterpret_cast<Cell**>(thingp));
     }
 
     Zone* zone = thing->asTenured().zoneFromAnyThread();
     if (zone->isGCSweeping()) {
         return IsAboutToBeFinalizedDuringSweep(thing->asTenured());
     } else if (zone->isGCCompacting() && IsForwarded(thing)) {
         *thingp = Forwarded(thing);
         return false;
--- a/js/src/gc/Nursery-inl.h
+++ b/js/src/gc/Nursery-inl.h
@@ -22,24 +22,24 @@
 template<typename T>
 bool
 js::Nursery::isInside(const SharedMem<T>& p) const
 {
     return isInside(p.unwrap(/*safe - used for value in comparison above*/));
 }
 
 MOZ_ALWAYS_INLINE /* static */ bool
-js::Nursery::getForwardedPointer(JSObject** ref)
+js::Nursery::getForwardedPointer(js::gc::Cell** ref)
 {
     MOZ_ASSERT(ref);
     MOZ_ASSERT(IsInsideNursery(*ref));
     const gc::RelocationOverlay* overlay = reinterpret_cast<const gc::RelocationOverlay*>(*ref);
     if (!overlay->isForwarded())
         return false;
-    *ref = static_cast<JSObject*>(overlay->forwardingAddress());
+    *ref = overlay->forwardingAddress();
     return true;
 }
 
 inline void
 js::Nursery::maybeSetForwardingPointer(JSTracer* trc, void* oldData, void* newData, bool direct)
 {
     if (trc->isTenuringTracer())
         setForwardingPointerWhileTenuring(oldData, newData, direct);
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -380,17 +380,17 @@ js::Nursery::allocateBuffer(Zone* zone, 
 
     if (nbytes <= MaxNurseryBufferSize) {
         void* buffer = allocate(nbytes);
         if (buffer)
             return buffer;
     }
 
     void* buffer = zone->pod_malloc<uint8_t>(nbytes);
-    if (buffer && !mallocedBuffers.putNew(buffer)) {
+    if (buffer && !registerMallocedBuffer(buffer)) {
         js_free(buffer);
         return nullptr;
     }
     return buffer;
 }
 
 void*
 js::Nursery::allocateBuffer(JSObject* obj, size_t nbytes)
@@ -503,18 +503,20 @@ js::Nursery::forwardBufferPointer(HeapSl
     MOZ_ASSERT(!isInside(*pSlotsElems));
     MOZ_ASSERT(IsWriteableAddress(*pSlotsElems));
 }
 
 js::TenuringTracer::TenuringTracer(JSRuntime* rt, Nursery* nursery)
   : JSTracer(rt, JSTracer::TracerKindTag::Tenuring, TraceWeakMapKeysValues)
   , nursery_(*nursery)
   , tenuredSize(0)
-  , head(nullptr)
-  , tail(&head)
+  , objHead(nullptr)
+  , objTail(&objHead)
+  , stringHead(nullptr)
+  , stringTail(&stringHead)
 {
 }
 
 inline float
 js::Nursery::calcPromotionRate(bool *validForTenuring) const {
     float used = float(previousGC.nurseryUsedBytes);
     float capacity = float(previousGC.nurseryCapacity);
     float tenured = float(previousGC.tenuredBytes);
@@ -902,16 +904,23 @@ js::Nursery::FreeMallocedBuffersTask::tr
 void
 js::Nursery::FreeMallocedBuffersTask::run()
 {
     for (MallocedBuffersSet::Range r = buffers_.all(); !r.empty(); r.popFront())
         fop_->free_(r.front());
     buffers_.clear();
 }
 
+bool
+js::Nursery::registerMallocedBuffer(void* buffer)
+{
+    MOZ_ASSERT(buffer);
+    return mallocedBuffers.putNew(buffer);
+}
+
 void
 js::Nursery::freeMallocedBuffers()
 {
     if (mallocedBuffers.empty())
         return;
 
     bool started;
     {
--- a/js/src/gc/Nursery.h
+++ b/js/src/gc/Nursery.h
@@ -75,46 +75,51 @@ class MacroAssembler;
 class TenuringTracer : public JSTracer
 {
     friend class Nursery;
     Nursery& nursery_;
 
     // Amount of data moved to the tenured generation during collection.
     size_t tenuredSize;
 
-    // This list is threaded through the Nursery using the space from already
-    // moved things. The list is used to fix up the moved things and to find
-    // things held live by intra-Nursery pointers.
-    gc::RelocationOverlay* head;
-    gc::RelocationOverlay** tail;
+    // These lists are threaded through the Nursery using the space from
+    // already moved things. The lists are used to fix up the moved things and
+    // to find things held live by intra-Nursery pointers.
+    gc::RelocationOverlay* objHead;
+    gc::RelocationOverlay** objTail;
+    gc::RelocationOverlay* stringHead;
+    gc::RelocationOverlay** stringTail;
 
     TenuringTracer(JSRuntime* rt, Nursery* nursery);
 
   public:
-    const Nursery& nursery() const { return nursery_; }
+    Nursery& nursery() { return nursery_; }
 
     template <typename T> void traverse(T** thingp);
     template <typename T> void traverse(T* thingp);
 
     // The store buffers need to be able to call these directly.
     void traceObject(JSObject* src);
     void traceObjectSlots(NativeObject* nobj, uint32_t start, uint32_t length);
     void traceSlots(JS::Value* vp, uint32_t nslots);
+    void traceString(JSString* src);
 
   private:
-    Nursery& nursery() { return nursery_; }
-
-    inline void insertIntoFixupList(gc::RelocationOverlay* entry);
+    inline void insertIntoObjectFixupList(gc::RelocationOverlay* entry);
+    inline void insertIntoStringFixupList(gc::RelocationOverlay* entry);
     template <typename T>
     inline T* allocTenured(JS::Zone* zone, gc::AllocKind kind);
 
     inline JSObject* movePlainObjectToTenured(PlainObject* src);
     JSObject* moveToTenuredSlow(JSObject* src);
+    JSString* moveToTenured(JSString* src);
+
     size_t moveElementsToTenured(NativeObject* dst, NativeObject* src, gc::AllocKind dstKind);
     size_t moveSlotsToTenured(NativeObject* dst, NativeObject* src, gc::AllocKind dstKind);
+    size_t moveStringToTenured(JSString* dst, JSString* src, gc::AllocKind dstKind);
 
     void traceSlots(JS::Value* vp, JS::Value* end);
 };
 
 /*
  * Classes with JSCLASS_SKIP_NURSERY_FINALIZE or Wrapper classes with
  * CROSS_COMPARTMENT flags will not have their finalizer called if they are
  * nursery allocated and not promoted to the tenured heap. The finalizers for
@@ -238,28 +243,35 @@ class Nursery
 
     /* The maximum number of bytes allowed to reside in nursery buffers. */
     static const size_t MaxNurseryBufferSize = 1024;
 
     /* Do a minor collection. */
     void collect(JS::gcreason::Reason reason);
 
     /*
-     * Check if the thing at |*ref| in the Nursery has been forwarded. If so,
-     * sets |*ref| to the new location of the object and returns true. Otherwise
-     * returns false and leaves |*ref| unset.
+     * If the thing at |*ref| in the Nursery has been forwarded, set |*ref| to
+     * the new location and return true. Otherwise return false and leave
+     * |*ref| unset.
      */
-    MOZ_ALWAYS_INLINE MOZ_MUST_USE static bool getForwardedPointer(JSObject** ref);
+    MOZ_ALWAYS_INLINE MOZ_MUST_USE static bool getForwardedPointer(js::gc::Cell** ref);
 
     /* Forward a slots/elements pointer stored in an Ion frame. */
     void forwardBufferPointer(HeapSlot** pSlotsElems);
 
     inline void maybeSetForwardingPointer(JSTracer* trc, void* oldData, void* newData, bool direct);
     inline void setForwardingPointerWhileTenuring(void* oldData, void* newData, bool direct);
 
+    /*
+     * Register a malloced buffer that is held by a nursery object, which
+     * should be freed at the end of a minor GC. Buffers are unregistered when
+     * their owning objects are tenured.
+     */
+    bool registerMallocedBuffer(void* buffer);
+
     /* Mark a malloced buffer as no longer needing to be freed. */
     void removeMallocedBuffer(void* buffer) {
         mallocedBuffers.remove(buffer);
     }
 
     void waitBackgroundFreeEnd();
 
     MOZ_MUST_USE bool addedUniqueIdToCell(gc::Cell* cell) {
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -821,17 +821,17 @@ JSCompartment::sweepAfterMinorGC(JSTrace
 {
     globalWriteBarriered = 0;
 
     InnerViewTable& table = innerViews.get();
     if (table.needsSweepAfterMinorGC())
         table.sweepAfterMinorGC();
 
     crossCompartmentWrappers.sweepAfterMinorGC(trc);
-
+    dtoaCache.purge();
     sweepMapAndSetObjectsAfterMinorGC();
 }
 
 void
 JSCompartment::sweepSavedStacks()
 {
     savedStacks_.sweep();
 }
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -229,16 +229,30 @@ JSFlatString::new_(JSContext* cx, const 
     JSFlatString* str;
     if (cx->compartment()->isAtomsCompartment())
         str = js::Allocate<js::NormalAtom, allowGC>(cx);
     else
         str = js::Allocate<JSFlatString, allowGC>(cx, js::gc::TenuredHeap);
     if (!str)
         return nullptr;
 
+    if (!str->isTenured()) {
+        // The chars pointer is only considered to be handed over to this
+        // function on a successful return. If the following registration
+        // fails, the string is partially initialized and must be made valid,
+        // or its finalizer may attempt to free uninitialized memory.
+        void* ptr = const_cast<void*>(static_cast<const void*>(chars));
+        if (!cx->runtime()->gc.nursery().registerMallocedBuffer(ptr)) {
+            str->init((JS::Latin1Char*)nullptr, 0);
+            if (allowGC)
+                ReportOutOfMemory(cx);
+            return nullptr;
+        }
+    }
+
     str->init(chars, length);
     return str;
 }
 
 inline js::PropertyName*
 JSFlatString::toPropertyName(JSContext* cx)
 {
 #ifdef DEBUG
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -512,16 +512,25 @@ JSRope::flattenInternal(JSContext* maybe
     }
 
     if (!AllocChars(this, wholeLength, &wholeChars, &wholeCapacity)) {
         if (maybecx)
             ReportOutOfMemory(maybecx);
         return nullptr;
     }
 
+    if (!isTenured() && maybecx) {
+        JSRuntime* rt = maybecx->runtime();
+        if (!rt->gc.nursery().registerMallocedBuffer(wholeChars)) {
+            js_free(wholeChars);
+            ReportOutOfMemory(maybecx);
+            return nullptr;
+        }
+    }
+
     pos = wholeChars;
     first_visit_node: {
         if (b == WithIncrementalBarrier) {
             JSString::writeBarrierPre(str->d.s.u2.left);
             JSString::writeBarrierPre(str->d.s.u3.right);
         }
 
         JSString& left = *str->d.s.u2.left;
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -685,16 +685,17 @@ class JSRope : public JSString
 
 static_assert(sizeof(JSRope) == sizeof(JSString),
               "string subclasses must be binary-compatible with JSString");
 
 class JSLinearString : public JSString
 {
     friend class JSString;
     friend class js::AutoStableStringChars;
+    friend class js::TenuringTracer;
 
     /* Vacuous and therefore unimplemented. */
     JSLinearString* ensureLinear(JSContext* cx) = delete;
     bool isLinear() const = delete;
     JSLinearString& asLinear() const = delete;
 
   protected:
     /* Returns void pointer to latin1/twoByte chars, for finalizers. */