From 1583dc99347684c1ec7864ce0ddac459ac221a24 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 22 Mar 2016 13:23:49 +0000 Subject: [PATCH] Bug 1257903 - Compact arenas containing shapes r=terrence --- js/src/builtin/TypedObject.cpp | 5 +- js/src/gc/GCInternals.h | 1 + js/src/gc/Heap.h | 6 +++ js/src/jsgc.cpp | 97 ++++++++++++++++++++-------------- js/src/jsgc.h | 31 +++++++++-- js/src/jsobj.cpp | 8 +-- js/src/jspropertytree.cpp | 17 ++++-- js/src/vm/NativeObject-inl.h | 8 +++ js/src/vm/NativeObject.h | 2 + js/src/vm/Shape.cpp | 49 +++++++++++++---- js/src/vm/Shape.h | 7 ++- 11 files changed, 166 insertions(+), 65 deletions(-) diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp index 82806ffd173c..07d9b2bf699b 100644 --- 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(); + return MaybeForwarded(&fieldDescrs.getDenseElement(index).toObject())->as(); } /****************************************************************************** @@ -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(), !owner->as().forInlineTypedObject()); if (owner != oldOwner && diff --git a/js/src/gc/GCInternals.h b/js/src/gc/GCInternals.h index 723636f41925..02aff832fbe2 100644 --- 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())); } diff --git a/js/src/gc/Heap.h b/js/src/gc/Heap.h index 50f5eb284341..6472dbda4d4d 100644 --- 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) { diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 16cd63138944..fb3c8e7662ba 100644 --- 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 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(); + if (baseShape->hasTable()) + baseShape->table().checkAfterMovingGC(); + } } for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) { c->objectGroups.checkTablesAfterMovingGC(); diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 826fcd34962d..4809208cb216 100644 --- 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 struct MightBeForwarded @@ -1094,7 +1106,8 @@ struct MightBeForwarded static_assert(!mozilla::IsSame::value && !mozilla::IsSame::value, "T must not be Cell or TenuredCell"); - static const bool value = mozilla::IsBaseOf::value; + static const bool value = mozilla::IsBaseOf::value || + mozilla::IsBaseOf::value; }; template @@ -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().updateShapeAfterMovingGC(); +} + template 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 diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index c6f548d15d4d..9939d7745bc2 100644 --- 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(); @@ -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* diff --git a/js/src/jspropertytree.cpp b/js/src/jspropertytree.cpp index 4982cf4d6952..5574c3632656 100644 --- a/js/src/jspropertytree.cpp +++ b/js/src/jspropertytree.cpp @@ -248,19 +248,20 @@ Shape::fixupDictionaryShapeAfterMovingGC() // alignment. Cell* cell = reinterpret_cast(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(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(uintptr_t(listp) - JSObject::offsetOfShape()); - listp = &gc::MaybeForwarded(last)->as().shape_; + if (gc::IsForwarded(last)) + listp = &gc::Forwarded(last)->as().shape_; } } @@ -317,6 +318,14 @@ Shape::fixupAfterMovingGC() fixupShapeTreeAfterMovingGC(); } +void +BaseShape::fixupAfterMovingGC() +{ + if (hasTable()) + table().fixupAfterMovingGC(); +} + + void Shape::fixupGetterSetterForBarrier(JSTracer* trc) { diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 66a9abbc064f..23aeb3d7bbd2 100644 --- 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) diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index d46ff9c4caf6..c2cd36dc8013 100644 --- 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() { diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp index 4dc0d2e1fd5d..9c10565d2ae0 100644 --- 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); } } diff --git a/js/src/vm/Shape.h b/js/src/vm/Shape.h index 14c194302524..2886ddf91f3c 100644 --- a/js/src/vm/Shape.h +++ b/js/src/vm/Shape.h @@ -228,6 +228,11 @@ class ShapeTable { template 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() {