Bug 1257903 - Compact arenas containing shapes r=terrence

2016-03-22 13:23:49 +00:00 · 2016-03-22 13:23:49 +00:00 · 1583dc9934
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@ -1083,9 +1083,7 @@ StructTypeDescr::maybeForwardedFieldDescr(size_t index) const
 {
    ArrayObject& fieldDescrs = *MaybeForwarded(&fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_TYPES));
    MOZ_ASSERT(index < fieldDescrs.getDenseInitializedLength());
-    JSObject& descr =
-        *MaybeForwarded(&fieldDescrs.getDenseElement(index).toObject());
-    return descr.as<TypeDescr>();
+    return MaybeForwarded(&fieldDescrs.getDenseElement(index).toObject())->as<TypeDescr>();
 }

 /******************************************************************************
@ -1659,6 +1657,7 @@ OutlineTypedObject::obj_trace(JSTracer* trc, JSObject* object)
    // inline with it. Note that an array buffer pointing to data in an inline
    // typed object will never be used as an owner for another outline typed
    // object. In such cases, the owner will be the inline typed object itself.
+    MakeAccessibleAfterMovingGC(owner);
    MOZ_ASSERT_IF(owner->is<ArrayBufferObject>(),
                  !owner->as<ArrayBufferObject>().forInlineTypedObject());
    if (owner != oldOwner &&
--- a/js/src/gc/GCInternals.h
+++ b/js/src/gc/GCInternals.h
@ -124,6 +124,7 @@ struct MovingTracer : JS::CallbackTracer
    explicit MovingTracer(JSRuntime* rt) : CallbackTracer(rt, TraceWeakMapKeysValues) {}

    void onObjectEdge(JSObject** objp) override;
+    void onShapeEdge(Shape** shapep) override;
    void onChild(const JS::GCCellPtr& thing) override {
        MOZ_ASSERT(!RelocationOverlay::isCellForwarded(thing.asCell()));
    }
--- a/js/src/gc/Heap.h
+++ b/js/src/gc/Heap.h
@ -124,6 +124,12 @@ IsObjectAllocKind(AllocKind kind)
    return kind >= AllocKind::OBJECT_FIRST && kind <= AllocKind::OBJECT_LAST;
 }

+inline bool
+IsShapeAllocKind(AllocKind kind)
+{
+    return kind == AllocKind::SHAPE || kind == AllocKind::ACCESSOR_SHAPE;
+}
+
 inline bool
 IsValidAllocKind(AllocKind kind)
 {
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@ -2021,11 +2021,24 @@ CanRelocateZone(Zone* zone)
    return !zone->isAtomsZone() && !zone->isSelfHostingZone();
 }

-static bool
-CanRelocateAllocKind(AllocKind kind)
-{
-    return IsObjectAllocKind(kind);
-}
+static const AllocKind AllocKindsToRelocate[] = {
+    AllocKind::FUNCTION,
+    AllocKind::FUNCTION_EXTENDED,
+    AllocKind::OBJECT0,
+    AllocKind::OBJECT0_BACKGROUND,
+    AllocKind::OBJECT2,
+    AllocKind::OBJECT2_BACKGROUND,
+    AllocKind::OBJECT4,
+    AllocKind::OBJECT4_BACKGROUND,
+    AllocKind::OBJECT8,
+    AllocKind::OBJECT8_BACKGROUND,
+    AllocKind::OBJECT12,
+    AllocKind::OBJECT12_BACKGROUND,
+    AllocKind::OBJECT16,
+    AllocKind::OBJECT16_BACKGROUND,
+    AllocKind::SHAPE,
+    AllocKind::ACCESSOR_SHAPE
+};

 Arena*
 ArenaList::removeRemainingArenas(Arena** arenap)
@ -2295,33 +2308,29 @@ ArenaLists::relocateArenas(Zone* zone, Arena*& relocatedListOut, JS::gcreason::R

    if (ShouldRelocateAllArenas(reason)) {
        zone->prepareForCompacting();
-        for (auto i : AllAllocKinds()) {
-            if (CanRelocateAllocKind(i)) {
-                ArenaList& al = arenaLists[i];
-                Arena* allArenas = al.head();
-                al.clear();
-                relocatedListOut = al.relocateArenas(allArenas, relocatedListOut, sliceBudget, stats);
-            }
+        for (auto kind : AllocKindsToRelocate) {
+            ArenaList& al = arenaLists[kind];
+            Arena* allArenas = al.head();
+            al.clear();
+            relocatedListOut = al.relocateArenas(allArenas, relocatedListOut, sliceBudget, stats);
        }
    } else {
        size_t arenaCount = 0;
        size_t relocCount = 0;
        AllAllocKindArray<Arena**> toRelocate;

-        for (auto i : AllAllocKinds()) {
-            toRelocate[i] = nullptr;
-            if (CanRelocateAllocKind(i))
-                toRelocate[i] = arenaLists[i].pickArenasToRelocate(arenaCount, relocCount);
+        for (auto kind : AllocKindsToRelocate) {
+            toRelocate[kind] = arenaLists[kind].pickArenasToRelocate(arenaCount, relocCount);
        }

        if (!ShouldRelocateZone(arenaCount, relocCount, reason))
            return false;

        zone->prepareForCompacting();
-        for (auto i : AllAllocKinds()) {
-            if (toRelocate[i]) {
-                ArenaList& al = arenaLists[i];
-                Arena* arenas = al.removeRemainingArenas(toRelocate[i]);
+        for (auto kind : AllocKindsToRelocate) {
+            if (toRelocate[kind]) {
+                ArenaList& al = arenaLists[kind];
+                Arena* arenas = al.removeRemainingArenas(toRelocate[kind]);
                relocatedListOut = al.relocateArenas(arenas, relocatedListOut, sliceBudget, stats);
            }
        }
@ -2347,15 +2356,12 @@ GCRuntime::relocateArenas(Zone* zone, JS::gcreason::Reason reason, Arena*& reloc
 #ifdef DEBUG
    // Check that we did as much compaction as we should have. There
    // should always be less than one arena's worth of free cells.
-    for (auto i : AllAllocKinds()) {
-        size_t thingsPerArena = Arena::thingsPerArena(i);
-        if (CanRelocateAllocKind(i)) {
-            ArenaList& al = zone->arenas.arenaLists[i];
-            size_t freeCells = 0;
-            for (Arena* arena = al.arenaAfterCursor(); arena; arena = arena->next)
-                freeCells += arena->countFreeCells();
-            MOZ_ASSERT(freeCells < thingsPerArena);
-        }
+    for (auto i : AllocKindsToRelocate) {
+        ArenaList& al = zone->arenas.arenaLists[i];
+        size_t freeCells = 0;
+        for (Arena* arena = al.arenaAfterCursor(); arena; arena = arena->next)
+            freeCells += arena->countFreeCells();
+        MOZ_ASSERT(freeCells < Arena::thingsPerArena(i));
    }
 #endif

@ -2365,8 +2371,17 @@ GCRuntime::relocateArenas(Zone* zone, JS::gcreason::Reason reason, Arena*& reloc
 void
 MovingTracer::onObjectEdge(JSObject** objp)
 {
-    if (IsForwarded(*objp))
-        *objp = Forwarded(*objp);
+    JSObject* obj = *objp;
+    if (IsForwarded(obj))
+        *objp = Forwarded(obj);
+}
+
+void
+MovingTracer::onShapeEdge(Shape** shapep)
+{
+    Shape* shape = *shapep;
+    if (IsForwarded(shape))
+        *shapep = Forwarded(shape);
 }

 void
@ -2538,11 +2553,8 @@ ArenasToUpdate::updateKind(AllocKind kind)
    //  - we assume JSObjects that are foreground finalized are not safe to
    //    update in parallel
    //  - updating a shape touches child shapes in fixupShapeTreeAfterMovingGC()
-    if (!js::gc::IsBackgroundFinalized(kind) ||
-        kind == AllocKind::SHAPE || kind == AllocKind::ACCESSOR_SHAPE)
-    {
+    if (!js::gc::IsBackgroundFinalized(kind) || IsShapeAllocKind(kind))
        return FOREGROUND;
-    }

    return BACKGROUND;
 }
@ -2726,6 +2738,11 @@ GCRuntime::updatePointersToRelocatedCells(Zone* zone)
        comp->fixupAfterMovingGC();
    JSCompartment::fixupCrossCompartmentWrappersAfterMovingGC(&trc);

+    // Iterate through all cells that can contain relocatable pointers to update
+    // them. Since updating each cell is independent we try to parallelize this
+    // as much as possible.
+    updateAllCellPointers(&trc, zone);
+
    // Mark roots to update them.
    {
        markRuntime(&trc, MarkRuntime);
@ -2765,11 +2782,6 @@ GCRuntime::updatePointersToRelocatedCells(Zone* zone)
    callWeakPointerZoneGroupCallbacks();
    for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next())
        callWeakPointerCompartmentCallbacks(comp);
-
-    // Finally, iterate through all cells that can contain JSObject pointers to
-    // update them. Since updating each cell is independent we try to
-    // parallelize this as much as possible.
-    updateAllCellPointers(&trc, zone);
 }

 void
@ -7331,6 +7343,11 @@ js::gc::CheckHashTablesAfterMovingGC(JSRuntime* rt)
     */
    for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) {
        zone->checkUniqueIdTableAfterMovingGC();
+        for (ZoneCellIter i(zone, AllocKind::BASE_SHAPE); !i.done(); i.next()) {
+            BaseShape* baseShape = i.get<BaseShape>();
+            if (baseShape->hasTable())
+                baseShape->table().checkAfterMovingGC();
+        }
    }
    for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) {
        c->objectGroups.checkTablesAfterMovingGC();
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@ -1084,7 +1084,19 @@ class RelocationOverlay
    }
 };

-/* Functions for checking and updating things that might be moved by compacting GC. */
+// Functions for checking and updating GC thing pointers that might have been
+// moved by compacting GC. Overloads are also provided that work with Values.
+//
+// IsForwarded    - check whether a pointer refers to an GC thing that has been
+//                  moved.
+//
+// Forwarded      - return a pointer to the new location of a GC thing given a
+//                  pointer to old location.
+//
+// MaybeForwarded - used before dereferencing a pointer that may refer to a
+//                  moved GC thing without updating it. For JSObjects this will
+//                  also update the object's shape pointer if it has been moved
+//                  to allow slots to be accessed.

 template <typename T>
 struct MightBeForwarded
@ -1094,7 +1106,8 @@ struct MightBeForwarded
    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;
+    static const bool value = mozilla::IsBaseOf<JSObject, T>::value ||
+                              mozilla::IsBaseOf<Shape, T>::value;
 };

 template <typename T>
@ -1141,11 +1154,23 @@ Forwarded(const JS::Value& value)
    return DispatchTyped(ForwardedFunctor(), value);
 }

+inline void
+MakeAccessibleAfterMovingGC(void* anyp) {}
+
+inline void
+MakeAccessibleAfterMovingGC(JSObject* obj) {
+    if (obj->isNative())
+        obj->as<NativeObject>().updateShapeAfterMovingGC();
+}
+
 template <typename T>
 inline T
 MaybeForwarded(T t)
 {
-    return IsForwarded(t) ? Forwarded(t) : t;
+    if (IsForwarded(t))
+        t = Forwarded(t);
+    MakeAccessibleAfterMovingGC(t);
+    return t;
 }

 #ifdef JSGC_HASH_TABLE_CHECKS
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@ -3790,9 +3790,6 @@ JSObject::traceChildren(JSTracer* trc)
    TraceEdge(trc, &group_, "group");

    const Class* clasp = group_->clasp();
-    if (clasp->trace)
-        clasp->trace(trc, this);
-
    if (clasp->isNative()) {
        NativeObject* nobj = &as<NativeObject>();

@ -3828,6 +3825,11 @@ JSObject::traceChildren(JSTracer* trc)
                       "objectElements");
        } while (false);
    }
+
+    // Call the trace hook at the end so that during a moving GC the trace hook
+    // will see updated fields and slots.
+    if (clasp->trace)
+        clasp->trace(trc, this);
 }

 static JSAtom*
--- a/js/src/jspropertytree.cpp
+++ b/js/src/jspropertytree.cpp
@ -248,19 +248,20 @@ Shape::fixupDictionaryShapeAfterMovingGC()
    // alignment.
    Cell* cell = reinterpret_cast<Cell*>(uintptr_t(listp) & ~CellMask);
    AllocKind kind = TenuredCell::fromPointer(cell)->getAllocKind();
-    MOZ_ASSERT_IF(listpPointsIntoShape,
-                  kind == AllocKind::SHAPE || kind == AllocKind::ACCESSOR_SHAPE);
+    MOZ_ASSERT_IF(listpPointsIntoShape, IsShapeAllocKind(kind));
    MOZ_ASSERT_IF(!listpPointsIntoShape, IsObjectAllocKind(kind));
 #endif

    if (listpPointsIntoShape) {
        // listp points to the parent field of the next shape.
        Shape* next = reinterpret_cast<Shape*>(uintptr_t(listp) - offsetof(Shape, parent));
-        listp = &gc::MaybeForwarded(next)->parent;
+        if (gc::IsForwarded(next))
+            listp = &gc::Forwarded(next)->parent;
    } else {
        // listp points to the shape_ field of an object.
        JSObject* last = reinterpret_cast<JSObject*>(uintptr_t(listp) - JSObject::offsetOfShape());
-        listp = &gc::MaybeForwarded(last)->as<NativeObject>().shape_;
+        if (gc::IsForwarded(last))
+            listp = &gc::Forwarded(last)->as<NativeObject>().shape_;
    }
 }

@ -317,6 +318,14 @@ Shape::fixupAfterMovingGC()
        fixupShapeTreeAfterMovingGC();
 }

+void
+BaseShape::fixupAfterMovingGC()
+{
+    if (hasTable())
+        table().fixupAfterMovingGC();
+}
+
+
 void
 Shape::fixupGetterSetterForBarrier(JSTracer* trc)
 {
--- a/js/src/vm/NativeObject-inl.h
+++ b/js/src/vm/NativeObject-inl.h
@ -286,6 +286,14 @@ NativeObject::setSlotWithType(ExclusiveContext* cx, Shape* shape,
    AddTypePropertyId(cx, this, shape->propid(), value);
 }

+inline void
+NativeObject::updateShapeAfterMovingGC()
+{
+    Shape* shape = shape_.unbarrieredGet();
+    if (IsForwarded(shape))
+        shape_.unsafeSet(Forwarded(shape));
+}
+
 /* Make an object with pregenerated shape from a NEWOBJECT bytecode. */
 static inline PlainObject*
 CopyInitializerObject(JSContext* cx, HandlePlainObject baseobj, NewObjectKind newKind = GenericObject)
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@ -1253,6 +1253,8 @@ class NativeObject : public JSObject
    copy(ExclusiveContext* cx, gc::AllocKind kind, gc::InitialHeap heap,
         HandleNativeObject templateObject);

+    void updateShapeAfterMovingGC();
+
    /* JIT Accessors */
    static size_t offsetOfElements() { return offsetof(NativeObject, elements_); }
    static size_t offsetOfFixedElements() {
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@ -328,6 +328,32 @@ ShapeTable::grow(ExclusiveContext* cx)
    return true;
 }

+void
+ShapeTable::fixupAfterMovingGC()
+{
+    for (size_t i = 0; i < capacity(); i++) {
+        Entry& entry = getEntry(i);
+        Shape* shape = entry.shape();
+        if (shape && IsForwarded(shape))
+            entry.setPreservingCollision(Forwarded(shape));
+    }
+}
+
+#ifdef JSGC_HASH_TABLE_CHECKS
+
+void
+ShapeTable::checkAfterMovingGC()
+{
+    for (size_t i = 0; i < capacity(); i++) {
+        Entry& entry = getEntry(i);
+        Shape* shape = entry.shape();
+        if (shape)
+            CheckGCThingAfterMovingGC(shape);
+    }
+}
+
+#endif
+
 /* static */ Shape*
 Shape::replaceLastProperty(ExclusiveContext* cx, StackBaseShape& base,
                           TaggedProto proto, HandleShape shape)
@ -1403,6 +1429,7 @@ JSCompartment::checkInitialShapesTableAfterMovingGC()
        TaggedProto proto = entry.proto.unbarrieredGet();
        Shape* shape = entry.shape.unbarrieredGet();

+        CheckGCThingAfterMovingGC(shape);
        if (proto.isObject())
            CheckGCThingAfterMovingGC(proto.toObject());

@ -1556,21 +1583,21 @@ JSCompartment::fixupInitialShapeTable()
        return;

    for (InitialShapeSet::Enum e(initialShapes); !e.empty(); e.popFront()) {
-        InitialShapeEntry entry = e.front();
-        bool needRekey = false;
-        if (IsForwarded(entry.shape.unbarrieredGet())) {
-            entry.shape.set(Forwarded(entry.shape.unbarrieredGet()));
-            needRekey = true;
+        // The shape may have been moved, but we can update that in place.
+        Shape* shape = e.front().shape.unbarrieredGet();
+        if (IsForwarded(shape)) {
+            shape = Forwarded(shape);
+            e.mutableFront().shape.set(shape);
        }
+
+        // If the prototype has moved we have to rekey the entry.
+        InitialShapeEntry entry = e.front();
        if (entry.proto.isObject() && IsForwarded(entry.proto.toObject())) {
            entry.proto = TaggedProto(Forwarded(entry.proto.toObject()));
-            needRekey = true;
-        }
-        if (needRekey) {
-            InitialShapeEntry::Lookup relookup(entry.shape.unbarrieredGet()->getObjectClass(),
+            InitialShapeEntry::Lookup relookup(shape->getObjectClass(),
                                               entry.proto,
-                                               entry.shape.unbarrieredGet()->numFixedSlots(),
-                                               entry.shape.unbarrieredGet()->getObjectFlags());
+                                               shape->numFixedSlots(),
+                                               shape->getObjectFlags());
            e.rekeyFront(relookup, entry);
        }
    }
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@ -228,6 +228,11 @@ class ShapeTable {
    template<MaybeAdding Adding>
    Entry& search(jsid id);

+    void fixupAfterMovingGC();
+#ifdef JSGC_HASH_TABLE_CHECKS
+    void checkAfterMovingGC();
+#endif
+
  private:
    Entry& getEntry(uint32_t i) const {
        MOZ_ASSERT(i < capacity());
@ -450,7 +455,7 @@ class BaseShape : public gc::TenuredCell

    void traceChildren(JSTracer* trc);

-    void fixupAfterMovingGC() {}
+    void fixupAfterMovingGC();

  private:
    static void staticAsserts() {