diff --git a/js/public/HeapAPI.h b/js/public/HeapAPI.h index d91ef634f0e2..27ede0c105c5 100644 --- a/js/public/HeapAPI.h +++ b/js/public/HeapAPI.h @@ -525,39 +525,25 @@ static MOZ_ALWAYS_INLINE JS::Zone* GetTenuredGCThingZone(const uintptr_t addr) { return *reinterpret_cast(zone_addr); } -static MOZ_ALWAYS_INLINE bool TenuredCellIsMarkedBlack( - const TenuredCell* cell) { - // Return true if BlackBit is set. - +static MOZ_ALWAYS_INLINE bool TenuredCellIsMarkedGray(const TenuredCell* cell) { + // Return true if GrayOrBlackBit is set and BlackBit is not set. MOZ_ASSERT(cell); MOZ_ASSERT(!js::gc::IsInsideNursery(cell)); - MarkBitmapWord* blackWord; - uintptr_t blackMask; - TenuredChunkBase* chunk = GetCellChunkBase(cell); - chunk->markBits.getMarkWordAndMask(cell, js::gc::ColorBit::BlackBit, - &blackWord, &blackMask); - return *blackWord & blackMask; -} - -static MOZ_ALWAYS_INLINE bool NonBlackCellIsMarkedGray( - const TenuredCell* cell) { - // Return true if GrayOrBlackBit is set. Callers should check BlackBit first. - - MOZ_ASSERT(cell); - MOZ_ASSERT(!js::gc::IsInsideNursery(cell)); - MOZ_ASSERT(!TenuredCellIsMarkedBlack(cell)); - MarkBitmapWord* grayWord; uintptr_t grayMask; TenuredChunkBase* chunk = GetCellChunkBase(cell); chunk->markBits.getMarkWordAndMask(cell, js::gc::ColorBit::GrayOrBlackBit, &grayWord, &grayMask); - return *grayWord & grayMask; -} + if (!(*grayWord & grayMask)) { + return false; + } -static MOZ_ALWAYS_INLINE bool TenuredCellIsMarkedGray(const TenuredCell* cell) { - return !TenuredCellIsMarkedBlack(cell) && NonBlackCellIsMarkedGray(cell); + MarkBitmapWord* blackWord; + uintptr_t blackMask; + chunk->markBits.getMarkWordAndMask(cell, js::gc::ColorBit::BlackBit, + &blackWord, &blackMask); + return !(*blackWord & blackMask); } static MOZ_ALWAYS_INLINE bool CellIsMarkedGray(const Cell* cell) { @@ -722,45 +708,24 @@ static MOZ_ALWAYS_INLINE void ExposeGCThingToActiveJS(JS::GCCellPtr thing) { return; } + auto* cell = reinterpret_cast(thing.asCell()); + // There's nothing to do for permanent GC things that might be owned by // another runtime. if (thing.mayBeOwnedByOtherRuntime()) { return; } - // Bug 1734801: I'd like to arrange for this to subsume the permanent GC thing - // check above. - auto* cell = reinterpret_cast(thing.asCell()); - if (detail::TenuredCellIsMarkedBlack(cell)) { - return; - } - auto* zone = JS::shadow::Zone::from(JS::GetTenuredGCThingZone(thing)); if (zone->needsIncrementalBarrier()) { PerformIncrementalReadBarrier(thing); - } else if (!zone->isGCPreparing() && detail::NonBlackCellIsMarkedGray(cell)) { + } else if (!zone->isGCPreparing() && detail::TenuredCellIsMarkedGray(cell)) { MOZ_ALWAYS_TRUE(JS::UnmarkGrayGCThingRecursively(thing)); } MOZ_ASSERT_IF(!zone->isGCPreparing(), !detail::TenuredCellIsMarkedGray(cell)); } -static MOZ_ALWAYS_INLINE void IncrementalReadBarrier(JS::GCCellPtr thing) { - // This is a lighter version of ExposeGCThingToActiveJS that doesn't do gray - // unmarking. - - if (IsInsideNursery(thing.asCell()) || thing.mayBeOwnedByOtherRuntime()) { - return; - } - - auto* zone = JS::shadow::Zone::from(JS::GetTenuredGCThingZone(thing)); - auto* cell = reinterpret_cast(thing.asCell()); - if (zone->needsIncrementalBarrier() && - !detail::TenuredCellIsMarkedBlack(cell)) { - PerformIncrementalReadBarrier(thing); - } -} - template extern JS_PUBLIC_API bool EdgeNeedsSweepUnbarrieredSlow(T* thingp); diff --git a/js/public/Id.h b/js/public/Id.h index bb32d21ca2f4..3bc2cb0bfaf2 100644 --- a/js/public/Id.h +++ b/js/public/Id.h @@ -271,11 +271,6 @@ struct BarrierMethods { js::gc::ExposeGCThingToActiveJS(id.toGCCellPtr()); } } - static void readBarrier(jsid id) { - if (id.isGCThing()) { - js::gc::IncrementalReadBarrier(id.toGCCellPtr()); - } - } }; // If the jsid is a GC pointer type, convert to that type and call |f| with the diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index ea387ddad77d..96caf7299f2c 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -312,11 +312,11 @@ class MOZ_NON_MEMMOVABLE Heap : public js::HeapOperations> { * breaks common usage of move semantics, so we need to define both, even * though they are equivalent. */ - explicit Heap(const Heap& other) { init(other.getWithoutExpose()); } - Heap(Heap&& other) { init(other.getWithoutExpose()); } + explicit Heap(const Heap& other) { init(other.ptr); } + Heap(Heap&& other) { init(other.ptr); } Heap& operator=(Heap&& other) { - set(other.getWithoutExpose()); + set(other.unbarrieredGet()); other.set(SafelyInitialized()); return *this; } @@ -329,15 +329,10 @@ class MOZ_NON_MEMMOVABLE Heap : public js::HeapOperations> { const T* address() const { return &ptr; } void exposeToActiveJS() const { js::BarrierMethods::exposeToJS(ptr); } - const T& get() const { exposeToActiveJS(); return ptr; } - const T& getWithoutExpose() const { - js::BarrierMethods::readBarrier(ptr); - return ptr; - } const T& unbarrieredGet() const { return ptr; } void set(const T& newPtr) { @@ -755,11 +750,6 @@ struct PtrBarrierMethodsBase { js::gc::ExposeGCThingToActiveJS(JS::GCCellPtr(t)); } } - static void readBarrier(T* t) { - if (t) { - js::gc::IncrementalReadBarrier(JS::GCCellPtr(t)); - } - } }; } // namespace detail diff --git a/js/public/Value.h b/js/public/Value.h index 0a9547dfd05f..fa6d8f6123e2 100644 --- a/js/public/Value.h +++ b/js/public/Value.h @@ -1106,11 +1106,6 @@ struct BarrierMethods { JS::HeapValuePostWriteBarrier(v, prev, next); } static void exposeToJS(const JS::Value& v) { JS::ExposeValueToActiveJS(v); } - static void readBarrier(const JS::Value& v) { - if (v.isGCThing()) { - js::gc::IncrementalReadBarrier(v.toGCCellPtr()); - } - } }; template diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 2e5cd1832134..968fe5e3f9ac 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -971,14 +971,12 @@ JS_PUBLIC_API void js::gc::PerformIncrementalReadBarrier(JS::GCCellPtr thing) { // Optimized marking for read barriers. This is called from // ExposeGCThingToActiveJS which has already checked the prerequisites for // performing a read barrier. This means we can skip a bunch of checks and - // call into the tracer directly. + // call info the tracer directly. MOZ_ASSERT(thing); MOZ_ASSERT(!JS::RuntimeHeapIsMajorCollecting()); TenuredCell* cell = &thing.asCell()->asTenured(); - MOZ_ASSERT(!cell->isMarkedBlack()); - Zone* zone = cell->zone(); MOZ_ASSERT(zone->needsIncrementalBarrier()); @@ -994,10 +992,6 @@ void js::gc::PerformIncrementalBarrier(TenuredCell* cell) { MOZ_ASSERT(cell); MOZ_ASSERT(!JS::RuntimeHeapIsMajorCollecting()); - if (cell->isMarkedBlack()) { - return; - } - Zone* zone = cell->zone(); MOZ_ASSERT(zone->needsIncrementalBarrier()); @@ -3169,10 +3163,11 @@ void BarrierTracer::performBarrier(JS::GCCellPtr cell) { MOZ_ASSERT(CurrentThreadCanAccessRuntime(runtime())); MOZ_ASSERT(!runtime()->gc.isBackgroundMarking()); MOZ_ASSERT(!cell.asCell()->isForwarded()); - MOZ_ASSERT(!cell.asCell()->asTenured().isMarkedBlack()); // Mark the cell here to prevent us recording it again. - cell.asCell()->asTenured().markBlack(); + if (!cell.asCell()->asTenured().markIfUnmarked()) { + return; + } // NOTE: This assumes that cells that don't have children do not require their // traceChildren method to be called. diff --git a/js/src/jsapi-tests/testGCHeapBarriers.cpp b/js/src/jsapi-tests/testGCHeapBarriers.cpp index 8508a302d37e..5cfc5650753d 100644 --- a/js/src/jsapi-tests/testGCHeapBarriers.cpp +++ b/js/src/jsapi-tests/testGCHeapBarriers.cpp @@ -60,7 +60,7 @@ JSFunction* CreateNurseryGCThing(JSContext* cx) { } template -static T CreateTenuredGCThing(JSContext* cx) { +static T* CreateTenuredGCThing(JSContext* cx) { MOZ_CRASH(); return nullptr; } @@ -346,26 +346,24 @@ BEGIN_TEST(testGCHeapReadBarriers) { CHECK((TestWrapperType, JSObject*>())); CHECK((TestWrapperType, JSObject*>())); - // JS::Heap has an additional barrier on its move and copy constructors. - CHECK((TestConstructorBarrier, JSObject*>())); - return true; } template bool TestWrapperType() { // Check that the read barrier normally marks gray things black. - CHECK((TestReadBarrierUnmarksGray())); - - // Check that the read barrier marks gray and white things black during an - // incremental GC. - CHECK((TestReadBarrierMarksBlack(true))); - CHECK((TestReadBarrierMarksBlack(false))); + { + Rooted obj0(cx, CreateTenuredGCThing(cx)); + WrapperT wrapper0(obj0); + MakeGray(obj0); + (void)*wrapper0; + CHECK(obj0->isMarkedBlack()); + } // Allocate test objects and make them gray. We will make sure they stay // gray. (For most reads, the barrier will unmark gray.) - Rooted obj1(cx, CreateTenuredGCThing(cx)); - Rooted obj2(cx, CreateTenuredGCThing(cx)); + Rooted obj1(cx, CreateTenuredGCThing(cx)); + Rooted obj2(cx, CreateTenuredGCThing(cx)); MakeGray(obj1); MakeGray(obj2); @@ -381,119 +379,6 @@ bool TestWrapperType() { return true; } -template -bool TestReadBarrierUnmarksGray() { - Rooted obj(cx, CreateTenuredGCThing(cx)); - WrapperT wrapper(obj); - - CHECK(obj->color() == gc::CellColor::White); - - (void)*wrapper; - - CHECK(obj->color() == gc::CellColor::White); - - MakeGray(obj); - (void)*wrapper; - - CHECK(obj->color() == gc::CellColor::Black); - - return true; -} - -// Execute thunk |f| between two slices of an incremental GC controlled by zeal -// mode |mode|. -template -bool CallDuringIncrementalGC(uint32_t mode, F&& f) { -#ifndef JS_GC_ZEAL - fprintf(stderr, "This test requires building with --enable-gczeal\n"); -#else - AutoGCParameter incremental(cx, JSGC_INCREMENTAL_GC_ENABLED, true); - - const int64_t BudgetMS = 10000; // 10S should be long enough for anyone. - - JS_SetGCZeal(cx, mode, 0); - JS::PrepareZoneForGC(cx, js::GetContextZone(cx)); - JS::StartIncrementalGC(cx, JS::GCOptions(), JS::GCReason::DEBUG_GC, BudgetMS); - CHECK(JS::IsIncrementalGCInProgress(cx)); - - CHECK(f()); - - JS::FinishIncrementalGC(cx, JS::GCReason::DEBUG_GC); -#endif - - return true; -} - -template -bool TestReadBarrierMarksBlack(bool fromWhite) { - AutoLeaveZeal noZeal(cx); - - // Create an object and hide it from the hazard analysis. - void* ptr = CreateTenuredGCThing(cx); - CHECK(ptr); - - CallDuringIncrementalGC(9 /* YieldBeforeSweeping */, [&]() -> bool { - CHECK(JS::IsIncrementalBarrierNeeded(cx)); - - auto obj = reinterpret_cast(ptr); - - WrapperT wrapper(obj); - - CHECK(obj->color() == gc::CellColor::White); - if (!fromWhite) { - MakeGray(obj); - } - - (void)*wrapper; - - CHECK(obj->color() == gc::CellColor::Black); - - return true; - }); - - return true; -} - -template -bool TestConstructorBarrier() { - AutoLeaveZeal noZeal(cx); - - // Create an object and hide it from the hazard analysis. - void* ptr = CreateTenuredGCThing(cx); - CHECK(ptr); - - CallDuringIncrementalGC(9 /* YieldBeforeSweeping */, [&]() -> bool { - CHECK(JS::IsIncrementalBarrierNeeded(cx)); - - auto obj = reinterpret_cast(ptr); - WrapperT wrapper(obj); - CHECK(obj->color() == gc::CellColor::White); - - WrapperT copiedWrapper(wrapper); - CHECK(obj->color() == gc::CellColor::Black); - - return true; - }); - - ptr = CreateTenuredGCThing(cx); - CHECK(ptr); - - CallDuringIncrementalGC(9 /* YieldBeforeSweeping */, [&]() -> bool { - CHECK(JS::IsIncrementalBarrierNeeded(cx)); - - auto obj = reinterpret_cast(ptr); - WrapperT wrapper(obj); - CHECK(obj->color() == gc::CellColor::White); - - WrapperT movedWrapper(std::move(wrapper)); - CHECK(obj->color() == gc::CellColor::Black); - - return true; - }); - - return true; -} - template bool TestUnbarrieredOperations(ObjectT obj, ObjectT obj2, WrapperT& wrapper, WrapperT& wrapper2) { @@ -559,7 +444,7 @@ BEGIN_TEST(testGCHeapPreBarriers) { size_t objectCount = 100; // Increase this if necessary when adding tests. ObjectVector testObjects; for (size_t i = 0; i < objectCount; i++) { - JSObject* obj = CreateTenuredGCThing(cx); + JSObject* obj = CreateTenuredGCThing(cx); CHECK(obj); CHECK(testObjects.append(obj)); } diff --git a/js/src/jsapi-tests/tests.h b/js/src/jsapi-tests/tests.h index 551ce0b9b3a3..198e1b2d9c25 100644 --- a/js/src/jsapi-tests/tests.h +++ b/js/src/jsapi-tests/tests.h @@ -542,7 +542,7 @@ class AutoLeaveZeal { JS_GetGCZealBits(cx_, &zealBits_, &frequency_, &dummy); JS_SetGCZeal(cx_, 0, 0); JS::PrepareForFullGC(cx_); - JS::NonIncrementalGC(cx_, JS::GCOptions::Normal, JS::GCReason::DEBUG_GC); + JS::NonIncrementalGC(cx_, JS::GCOptions::Shrink, JS::GCReason::DEBUG_GC); } ~AutoLeaveZeal() { JS_SetGCZeal(cx_, 0, 0); diff --git a/xpcom/tests/gtest/TestJSHolderMap.cpp b/xpcom/tests/gtest/TestJSHolderMap.cpp index 418b007d9efb..32b931b2ccfc 100644 --- a/xpcom/tests/gtest/TestJSHolderMap.cpp +++ b/xpcom/tests/gtest/TestJSHolderMap.cpp @@ -8,10 +8,8 @@ #include "mozilla/UniquePtr.h" #include "mozilla/Vector.h" +#include "nsISupportsUtils.h" #include "nsCycleCollectionParticipant.h" -#include "nsCycleCollector.h" - -#include "js/GCAPI.h" #include "gtest/gtest.h" @@ -175,121 +173,3 @@ TEST(JSHolderMap, TestAddRemoveMany) TestAddRemoveMany(SingleZone, 10000); TestAddRemoveMany(MultiZone, 10000); } - -class ObjectHolder final { - public: - ObjectHolder() { HoldJSObjects(this); } - - NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(ObjectHolder) - NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(ObjectHolder) - - void SetObject(JSObject* aObject) { mObject = aObject; } - - void ClearObject() { mObject = nullptr; } - - JSObject* GetObject() const { return mObject; } - JSObject* GetObjectUnbarriered() const { return mObject.unbarrieredGet(); } - - bool ObjectIsGray() const { - JSObject* obj = mObject.unbarrieredGet(); - MOZ_RELEASE_ASSERT(obj); - return JS::GCThingIsMarkedGray(JS::GCCellPtr(obj)); - } - - private: - JS::Heap mObject; - - ~ObjectHolder() { DropJSObjects(this); } -}; - -NS_IMPL_CYCLE_COLLECTION_CLASS(ObjectHolder) - -NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ObjectHolder) - tmp->ClearObject(); -NS_IMPL_CYCLE_COLLECTION_UNLINK_END - -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ObjectHolder) -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END - -NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ObjectHolder) - NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mObject) -NS_IMPL_CYCLE_COLLECTION_TRACE_END - -NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(ObjectHolder, AddRef) -NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(ObjectHolder, Release) - -// Test GC things stored in JS holders are marked as gray roots by the GC. -static void TestHoldersAreMarkedGray(JSContext* cx) { - RefPtr holder(new ObjectHolder); - - JSObject* obj = JS_NewPlainObject(cx); - ASSERT_TRUE(obj); - holder->SetObject(obj); - obj = nullptr; - - JS_GC(cx); - - ASSERT_TRUE(holder->ObjectIsGray()); -} - -// Test GC things stored in JS holders are updated by compacting GC. -static void TestHoldersAreMoved(JSContext* cx, bool singleZone) { - JS::RootedObject obj(cx, JS_NewPlainObject(cx)); - ASSERT_TRUE(obj); - - // Set a property so we can check we have the same object at the end. - const char* PropertyName = "answer"; - const int32_t PropertyValue = 42; - RootedValue value(cx, JS::Int32Value(PropertyValue)); - ASSERT_TRUE(JS_SetProperty(cx, obj, PropertyName, value)); - - // Ensure the object is tenured. - JS_GC(cx); - - RefPtr holder(new ObjectHolder); - holder->SetObject(obj); - - uintptr_t original = uintptr_t(obj.get()); - - if (singleZone) { - JS::PrepareZoneForGC(cx, js::GetContextZone(cx)); - } else { - JS::PrepareForFullGC(cx); - } - - JS::NonIncrementalGC(cx, JS::GCOptions::Shrink, JS::GCReason::DEBUG_GC); - - // Shrinking DEBUG_GC should move all GC things. - ASSERT_NE(uintptr_t(holder->GetObject()), original); - - // Both root and holder should have been updated. - ASSERT_EQ(obj, holder->GetObject()); - - // Check it's the object we expect. - value.setUndefined(); - ASSERT_TRUE(JS_GetProperty(cx, obj, PropertyName, &value)); - ASSERT_EQ(value, JS::Int32Value(PropertyValue)); -} - -TEST(JSHolderMap, GCIntegration) -{ - CycleCollectedJSContext* ccjscx = CycleCollectedJSContext::Get(); - ASSERT_NE(ccjscx, nullptr); - JSContext* cx = ccjscx->Context(); - ASSERT_NE(cx, nullptr); - - static const JSClass GlobalClass = {"global", JSCLASS_GLOBAL_FLAGS, - &JS::DefaultGlobalClassOps}; - - JS::RealmOptions options; - JS::RootedObject global(cx); - global = JS_NewGlobalObject(cx, &GlobalClass, nullptr, - JS::FireOnNewGlobalHook, options); - ASSERT_NE(global, nullptr); - - JSAutoRealm ar(cx, global); - - TestHoldersAreMarkedGray(cx); - TestHoldersAreMoved(cx, true); - TestHoldersAreMoved(cx, false); -}