tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit d33b4c513b87f48a62169adf23896d350a798d02
parent 4adc51ca69290dc0c33e00ba4be3afdcbe5ca0e2
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date:   Wed,  5 Nov 2025 12:49:18 +0000

Bug 1968172 - Skip tracing rooted vectors of non-nursery types in minor GC r=jandem

I noticed that for this test case the vector we're tracing repeatedly in
minor GC doesn't ever actually contain any nursery pointers.

The patch adds a base class to the GCPolicy template to add a default
implementation of mightBeInNursery(), returning true. A definition for
GCPolicy<jsid> overrides this since Ids can't be in the nursery. Finally we can
check this in StackGCVector::trace to skip tracing during minor GC for
RootedVectors.

This mostly removes the cost of nursery collection for the test case.

Differential Revision: https://phabricator.services.mozilla.com/D271248

Diffstat:
Mjs/public/GCPolicyAPI.h | 33+++++++++++++++++++--------------
Mjs/public/GCVector.h | 17+++++++++++++++++
Mjs/public/Id.h | 3++-
Mjs/public/Value.h | 2+-
Mjs/src/gc/NurseryAwareHashMap.h | 3++-
Mjs/src/gc/Policy.h | 12+++++++-----
6 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/js/public/GCPolicyAPI.h b/js/public/GCPolicyAPI.h @@ -81,12 +81,19 @@ namespace JS { +// Base class for GCPolicy<T> that provides some defaults. +template <typename T> +struct GCPolicyBase { + static bool isValid(const T& tp) { return true; /* Unchecked. */ } + static constexpr bool mightBeInNursery() { return true; /* Unknown. */ } +}; + // Defines a policy for container types with non-GC, i.e. C storage. This // policy dispatches to the underlying struct for GC interactions. Note that // currently a type can define only the subset of the methods (trace and/or // traceWeak) if it is never used in a context that requires the other. template <typename T> -struct StructGCPolicy { +struct StructGCPolicy : public GCPolicyBase<T> { static_assert( !std::is_pointer_v<T>, "Pointer types must have a GCPolicy<> specialization.\n" @@ -106,8 +113,6 @@ struct StructGCPolicy { static bool needsSweep(JSTracer* trc, const T* tp) { return tp->needsSweep(trc); } - - static bool isValid(const T& tp) { return true; } }; // The default GC policy attempts to defer to methods on the underlying type. @@ -119,11 +124,10 @@ struct GCPolicy : public StructGCPolicy<T> {}; // This policy ignores any GC interaction, e.g. for non-GC types. template <typename T> -struct IgnoreGCPolicy { +struct IgnoreGCPolicy : public GCPolicyBase<T> { static void trace(JSTracer* trc, T* t, const char* name) {} static bool traceWeak(JSTracer*, T* v) { return true; } static bool needsSweep(JSTracer* trc, const T* v) { return false; } - static bool isValid(const T& v) { return true; } }; template <> struct GCPolicy<uint8_t> : public IgnoreGCPolicy<uint8_t> {}; @@ -135,7 +139,7 @@ template <> struct GCPolicy<bool> : public IgnoreGCPolicy<bool> {}; template <typename T> -struct GCPointerPolicy { +struct GCPointerPolicy : public GCPolicyBase<T> { static_assert(std::is_pointer_v<T>, "Non-pointer type not allowed for GCPointerPolicy"); @@ -157,7 +161,7 @@ JS_FOR_EACH_PUBLIC_GC_POINTER_TYPE(EXPAND_SPECIALIZE_GCPOLICY) #undef EXPAND_SPECIALIZE_GCPOLICY template <typename T> -struct NonGCPointerPolicy { +struct NonGCPointerPolicy : public GCPolicyBase<T> { static void trace(JSTracer* trc, T* vp, const char* name) { if (*vp) { (*vp)->trace(trc); @@ -166,11 +170,10 @@ struct NonGCPointerPolicy { static bool traceWeak(JSTracer* trc, T* vp) { return !*vp || (*vp)->traceWeak(trc); } - static bool isValid(T v) { return true; } }; template <typename T> -struct GCPolicy<JS::Heap<T>> { +struct GCPolicy<JS::Heap<T>> : public GCPolicyBase<JS::Heap<T>> { static void trace(JSTracer* trc, JS::Heap<T>* thingp, const char* name) { TraceEdge(trc, thingp, name); } @@ -185,7 +188,8 @@ struct GCPolicy<JS::Heap<T>> { // GCPolicy<UniquePtr<T>> forwards the contained pointer to GCPolicy<T>. template <typename T, typename D> -struct GCPolicy<mozilla::UniquePtr<T, D>> { +struct GCPolicy<mozilla::UniquePtr<T, D>> + : public GCPolicyBase<mozilla::UniquePtr<T, D>> { static void trace(JSTracer* trc, mozilla::UniquePtr<T, D>* tp, const char* name) { if (tp->get()) { @@ -209,7 +213,7 @@ struct GCPolicy<mozilla::Nothing> : public IgnoreGCPolicy<mozilla::Nothing> {}; // GCPolicy<Maybe<T>> forwards tracing/sweeping to GCPolicy<T*> if // the Maybe<T> is filled and T* can be traced via GCPolicy<T*>. template <typename T> -struct GCPolicy<mozilla::Maybe<T>> { +struct GCPolicy<mozilla::Maybe<T>> : public GCPolicyBase<mozilla::Maybe<T>> { static void trace(JSTracer* trc, mozilla::Maybe<T>* tp, const char* name) { if (tp->isSome()) { GCPolicy<T>::trace(trc, tp->ptr(), name); @@ -227,7 +231,7 @@ struct GCPolicy<mozilla::Maybe<T>> { }; template <typename T1, typename T2> -struct GCPolicy<std::pair<T1, T2>> { +struct GCPolicy<std::pair<T1, T2>> : public GCPolicyBase<std::pair<T1, T2>> { static void trace(JSTracer* trc, std::pair<T1, T2>* tp, const char* name) { GCPolicy<T1>::trace(trc, &tp->first, name); GCPolicy<T2>::trace(trc, &tp->second, name); @@ -252,7 +256,8 @@ template <> struct GCPolicy<mozilla::Ok> : public IgnoreGCPolicy<mozilla::Ok> {}; template <typename V, typename E> -struct GCPolicy<mozilla::Result<V, E>> { +struct GCPolicy<mozilla::Result<V, E>> + : public GCPolicyBase<mozilla::Result<V, E>> { static void trace(JSTracer* trc, mozilla::Result<V, E>* tp, const char* name) { if (tp->isOk()) { @@ -272,7 +277,7 @@ struct GCPolicy<mozilla::Result<V, E>> { }; template <typename... Fs> -struct GCPolicy<std::tuple<Fs...>> { +struct GCPolicy<std::tuple<Fs...>> : public GCPolicyBase<std::tuple<Fs...>> { using T = std::tuple<Fs...>; static void trace(JSTracer* trc, T* tp, const char* name) { traceFieldsFrom<0>(trc, *tp, name); diff --git a/js/public/GCVector.h b/js/public/GCVector.h @@ -42,6 +42,7 @@ namespace JS { template <typename T, size_t MinInlineCapacity = 0, typename AllocPolicy = js::TempAllocPolicy> class GCVector { + protected: mozilla::Vector<T, MinInlineCapacity, AllocPolicy> vector; public: @@ -210,6 +211,22 @@ class MOZ_STACK_CLASS StackGCVector : public GCVector<T, 8, AllocPolicy> { public: using Base = GCVector<T, 8, AllocPolicy>; + void trace(JSTracer* trc) { + if constexpr (!GCPolicy<T>::mightBeInNursery()) { + // Skip tracing of non-nursery types in minor GC. + if (trc->isTenuringTracer()) { +#ifdef DEBUG + for (auto& elem : this->vector) { + MOZ_ASSERT(GCPolicy<T>::isTenured(elem)); + } +#endif + return; + } + } + + Base::trace(trc); + } + private: // Inherit constructor from GCVector. using Base::Base; diff --git a/js/public/Id.h b/js/public/Id.h @@ -234,7 +234,7 @@ namespace JS { extern JS_PUBLIC_DATA const JS::HandleId VoidHandlePropertyKey; template <> -struct GCPolicy<jsid> { +struct GCPolicy<jsid> : public GCPolicyBase<jsid> { static void trace(JSTracer* trc, jsid* idp, const char* name) { // This should only be called as part of root marking since that's the only // time we should trace unbarriered GC thing pointers. This will assert if @@ -246,6 +246,7 @@ struct GCPolicy<jsid> { js::gc::IsCellPointerValid(id.toGCCellPtr().asCell()); } + static constexpr bool mightBeInNursery() { return false; } static bool isTenured(jsid id) { MOZ_ASSERT_IF(id.isGCThing(), !js::gc::IsInsideNursery(id.toGCCellPtr().asCell())); diff --git a/js/public/Value.h b/js/public/Value.h @@ -1227,7 +1227,7 @@ JS_PUBLIC_API void HeapValueWriteBarriers(Value* valuep, const Value& prev, const Value& next); template <> -struct GCPolicy<JS::Value> { +struct GCPolicy<JS::Value> : public GCPolicyBase<JS::Value> { static void trace(JSTracer* trc, Value* v, const char* name) { // This should only be called as part of root marking since that's the only // time we should trace unbarriered GC thing pointers. This will assert if diff --git a/js/src/gc/NurseryAwareHashMap.h b/js/src/gc/NurseryAwareHashMap.h @@ -224,7 +224,8 @@ class NurseryAwareHashMap { namespace JS { template <typename T> -struct GCPolicy<js::detail::UnsafeBareWeakHeapPtr<T>> { +struct GCPolicy<js::detail::UnsafeBareWeakHeapPtr<T>> + : public GCPolicyBase<js::detail::UnsafeBareWeakHeapPtr<T>> { static void trace(JSTracer* trc, js::detail::UnsafeBareWeakHeapPtr<T>* thingp, const char* name) { js::TraceEdge(trc, thingp, name); diff --git a/js/src/gc/Policy.h b/js/src/gc/Policy.h @@ -49,7 +49,7 @@ template <typename T> struct GCPolicy<T* const> : public js::InternalGCPointerPolicy<T* const> {}; template <typename T> -struct GCPolicy<js::HeapPtr<T>> { +struct GCPolicy<js::HeapPtr<T>> : public GCPolicyBase<js::HeapPtr<T>> { static void trace(JSTracer* trc, js::HeapPtr<T>* thingp, const char* name) { js::TraceNullableEdge(trc, thingp, name); } @@ -65,7 +65,8 @@ struct GCPolicy<js::HeapPtr<T>> { }; template <typename T> -struct GCPolicy<js::PreBarriered<T>> { +struct GCPolicy<js::PreBarriered<T>> + : public GCPolicyBase<js::PreBarriered<T>> { static void trace(JSTracer* trc, js::PreBarriered<T>* thingp, const char* name) { js::TraceNullableEdge(trc, thingp, name); @@ -73,7 +74,7 @@ struct GCPolicy<js::PreBarriered<T>> { }; template <typename T> -struct GCPolicy<js::WeakHeapPtr<T>> { +struct GCPolicy<js::WeakHeapPtr<T>> : public GCPolicyBase<js::WeakHeapPtr<T>> { static void trace(JSTracer* trc, js::WeakHeapPtr<T>* thingp, const char* name) { js::TraceEdge(trc, thingp, name); @@ -90,7 +91,8 @@ struct GCPolicy<js::WeakHeapPtr<T>> { }; template <typename T> -struct GCPolicy<js::UnsafeBarePtr<T>> { +struct GCPolicy<js::UnsafeBarePtr<T>> + : public GCPolicyBase<js::UnsafeBarePtr<T>> { static bool traceWeak(JSTracer* trc, js::UnsafeBarePtr<T>* vp) { if (*vp) { return js::TraceManuallyBarrieredWeakEdge(trc, vp->unbarrieredAddress(), @@ -101,7 +103,7 @@ struct GCPolicy<js::UnsafeBarePtr<T>> { }; template <> -struct GCPolicy<JS::GCCellPtr> { +struct GCPolicy<JS::GCCellPtr> : public GCPolicyBase<JS::GCCellPtr> { static void trace(JSTracer* trc, JS::GCCellPtr* thingp, const char* name) { // It's not safe to trace unbarriered pointers except as part of root // marking.