From d4611b5958a387fc80f7493f689c3b6657d1610b Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Thu, 17 Sep 2015 10:57:55 -0700 Subject: [PATCH] Bug 1205454 - Consolidate the tagged pointer marking methods; r=sfink --- js/public/HeapAPI.h | 18 ++++ js/public/Id.h | 2 +- js/public/TraceKind.h | 2 +- js/public/Value.h | 2 +- js/src/gc/Barrier.h | 6 +- js/src/gc/Marking.cpp | 185 ++++++++++---------------------------- js/src/gc/Marking.h | 8 +- js/src/gc/Nursery.h | 1 + js/src/gc/RootMarking.cpp | 2 +- js/src/gc/Tracer.cpp | 6 +- js/src/jsgc.cpp | 4 +- js/src/jsgc.h | 6 +- js/src/vm/TaggedProto.h | 2 +- js/src/vm/UbiNode.cpp | 6 +- 14 files changed, 92 insertions(+), 158 deletions(-) diff --git a/js/public/HeapAPI.h b/js/public/HeapAPI.h index 57c05b30fdf..2987b77356f 100644 --- a/js/public/HeapAPI.h +++ b/js/public/HeapAPI.h @@ -245,6 +245,24 @@ operator!=(const GCCellPtr& ptr1, const GCCellPtr& ptr2) return !(ptr1 == ptr2); } +// Unwraps the given GCCellPtr and calls the given functor with a template +// argument of the actual type of the pointer. +template +auto +DispatchTyped(F f, GCCellPtr thing, Args&&... args) + -> decltype(f(static_cast(nullptr), mozilla::Forward(args)...)) +{ + switch (thing.kind()) { +#define JS_EXPAND_DEF(name, type, _) \ + case JS::TraceKind::name: \ + return f(&thing.as(), mozilla::Forward(args)...); + JS_FOR_EACH_TRACEKIND(JS_EXPAND_DEF); +#undef JS_EXPAND_DEF + default: + MOZ_CRASH("Invalid trace kind in DispatchTyped for GCCellPtr."); + } +} + } /* namespace JS */ namespace js { diff --git a/js/public/Id.h b/js/public/Id.h index ed8e597b483..51c85183501 100644 --- a/js/public/Id.h +++ b/js/public/Id.h @@ -178,7 +178,7 @@ template <> struct GCMethods // the pointer. If the jsid is not a GC type, calls F::defaultValue. template auto -DispatchIdTyped(F f, jsid& id, Args&&... args) +DispatchTyped(F f, jsid& id, Args&&... args) -> decltype(f(static_cast(nullptr), mozilla::Forward(args)...)) { if (JSID_IS_STRING(id)) diff --git a/js/public/TraceKind.h b/js/public/TraceKind.h index f4db1e20d80..c7e1a40dac1 100644 --- a/js/public/TraceKind.h +++ b/js/public/TraceKind.h @@ -125,7 +125,7 @@ DispatchTraceKindTyped(F f, JS::TraceKind traceKind, Args&&... args) template auto DispatchTraceKindTyped(F f, void* thing, JS::TraceKind traceKind, Args&&... args) - -> decltype(f(reinterpret_cast(0), mozilla::Forward(args)...)) + -> decltype(f(static_cast(nullptr), mozilla::Forward(args)...)) { switch (traceKind) { #define JS_EXPAND_DEF(name, type, _) \ diff --git a/js/public/Value.h b/js/public/Value.h index fefc7dbc3ce..0bbaf3d11c8 100644 --- a/js/public/Value.h +++ b/js/public/Value.h @@ -1832,7 +1832,7 @@ class PersistentRootedBase : public MutableValueOperations auto -DispatchValueTyped(F f, const JS::Value& val, Args&&... args) +DispatchTyped(F f, const JS::Value& val, Args&&... args) -> decltype(f(static_cast(nullptr), mozilla::Forward(args)...)) { if (val.isString()) diff --git a/js/src/gc/Barrier.h b/js/src/gc/Barrier.h index 7be7bfcc1a9..ea49af29bc8 100644 --- a/js/src/gc/Barrier.h +++ b/js/src/gc/Barrier.h @@ -261,7 +261,7 @@ struct InternalGCMethods static bool isMarkableTaggedPointer(Value v) { return isMarkable(v); } static void preBarrier(Value v) { - DispatchValueTyped(PreBarrierFunctor(), v); + DispatchTyped(PreBarrierFunctor(), v); } static void postBarrier(Value* vp, const Value& prev, const Value& next) { @@ -286,7 +286,7 @@ struct InternalGCMethods } static void readBarrier(const Value& v) { - DispatchValueTyped(ReadBarrierFunctor(), v); + DispatchTyped(ReadBarrierFunctor(), v); } }; @@ -296,7 +296,7 @@ struct InternalGCMethods static bool isMarkable(jsid id) { return JSID_IS_STRING(id) || JSID_IS_SYMBOL(id); } static bool isMarkableTaggedPointer(jsid id) { return isMarkable(id); } - static void preBarrier(jsid id) { DispatchIdTyped(PreBarrierFunctor(), id); } + static void preBarrier(jsid id) { DispatchTyped(PreBarrierFunctor(), id); } static void postBarrier(jsid* idp, jsid prev, jsid next) {} }; diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index ac7ec7ad1c6..f11c621d04c 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -172,7 +172,7 @@ template <> bool ThingIsPermanentAtomOrWellKnownSymbol(JS::Symbol* s template void -js::CheckTracedThing(JSTracer* trc, T thing) +js::CheckTracedThing(JSTracer* trc, T* thing) { #ifdef DEBUG MOZ_ASSERT(trc); @@ -242,30 +242,16 @@ struct CheckTracedFunctor : public VoidDefaultAdaptor { template void operator()(T* t, JSTracer* trc) { CheckTracedThing(trc, t); } }; +template +void +js::CheckTracedThing(JSTracer* trc, T thing) +{ + DispatchTyped(CheckTracedFunctor(), thing, trc); +} + namespace js { -template<> -void -CheckTracedThing(JSTracer* trc, Value val) -{ - DispatchValueTyped(CheckTracedFunctor(), val, trc); -} - -template <> -void -CheckTracedThing(JSTracer* trc, jsid id) -{ - DispatchIdTyped(CheckTracedFunctor(), id, trc); -} - -template <> -void -CheckTracedThing(JSTracer* trc, TaggedProto proto) -{ - DispatchTaggedProtoTyped(CheckTracedFunctor(), proto, trc); -} - #define IMPL_CHECK_TRACED_THING(_, type, __) \ - template void CheckTracedThing(JSTracer*, type*); + template void CheckTracedThing(JSTracer*, type*); JS_FOR_EACH_TRACEKIND(IMPL_CHECK_TRACED_THING); #undef IMPL_CHECK_TRACED_THING } // namespace js @@ -427,10 +413,7 @@ JS_FOR_EACH_TRACEKIND(IMPL_BASE_GC_TYPE); // share the definitions with Value and jsid. Thus, we need to strip the // pointer before sending the type to BaseGCType and re-add it on the other // side. As such: -template struct PtrBaseGCType {}; -template <> struct PtrBaseGCType { typedef Value type; }; -template <> struct PtrBaseGCType { typedef jsid type; }; -template <> struct PtrBaseGCType { typedef TaggedProto type; }; +template struct PtrBaseGCType { typedef T type; }; template struct PtrBaseGCType { typedef typename BaseGCType::type* type; }; template @@ -442,6 +425,7 @@ ConvertToBase(T* thingp) template void DispatchToTracer(JSTracer* trc, T* thingp, const char* name); template T DoCallback(JS::CallbackTracer* trc, T* thingp, const char* name); +template void DoMarking(GCMarker* gcmarker, T* thing); template void DoMarking(GCMarker* gcmarker, T thing); template @@ -735,7 +719,7 @@ MustSkipMarking(JS::Symbol* sym) template void -DoMarking(GCMarker* gcmarker, T thing) +DoMarking(GCMarker* gcmarker, T* thing) { // Do per-type marking precondition checks. if (MustSkipMarking(thing)) @@ -753,26 +737,11 @@ struct DoMarkingFunctor : public VoidDefaultAdaptor { template void operator()(T* t, GCMarker* gcmarker) { DoMarking(gcmarker, t); } }; -template <> +template void -DoMarking(GCMarker* gcmarker, Value val) +DoMarking(GCMarker* gcmarker, T thing) { - DispatchValueTyped(DoMarkingFunctor(), val, gcmarker); -} - -template <> -void -DoMarking(GCMarker* gcmarker, jsid id) -{ - DispatchIdTyped(DoMarkingFunctor(), id, gcmarker); -} - -template <> -void -DoMarking(GCMarker* gcmarker, TaggedProto proto) -{ - if (proto.isObject()) - DoMarking(gcmarker, proto.toObject()); + DispatchTyped(DoMarkingFunctor(), thing, gcmarker); } // The simplest traversal calls out to the fully generic traceChildren function @@ -843,7 +812,7 @@ GCMarker::traverse(AccessorShape* thing) { template void -js::GCMarker::traverseEdge(S source, T target) +js::GCMarker::traverseEdge(S source, T* target) { // Atoms and Symbols do not have or mark their internal pointers, respectively. MOZ_ASSERT(!ThingIsPermanentAtomOrWellKnownSymbol(source)); @@ -869,18 +838,11 @@ template struct TraverseEdgeFunctor : public VoidDefaul } }; -template +template void -js::GCMarker::traverseEdge(S source, jsid id) +js::GCMarker::traverseEdge(S source, T thing) { - DispatchIdTyped(TraverseEdgeFunctor(), id, this, source); -} - -template -void -js::GCMarker::traverseEdge(S source, Value v) -{ - DispatchValueTyped(TraverseEdgeFunctor(), v, this, source); + DispatchTyped(TraverseEdgeFunctor(), thing, this, source); } template @@ -1897,6 +1859,12 @@ GCMarker::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const /*** Tenuring Tracer *****************************************************************************/ namespace js { +template +void +TenuringTracer::traverse(T** tp) +{ +} + template <> void TenuringTracer::traverse(JSObject** objp) @@ -1908,39 +1876,20 @@ TenuringTracer::traverse(JSObject** objp) *objp = moveToTenured(*objp); } -template <> +template +struct TenuringTraversalFunctor : public IdentityDefaultAdaptor { + template S operator()(T* t, TenuringTracer* trc) { + trc->traverse(&t); + return js::gc::RewrapTaggedPointer::wrap(t); + } +}; + +template void -TenuringTracer::traverse(Value* valp) +TenuringTracer::traverse(T* thingp) { - if (!valp->isObject()) - return; - - JSObject *obj = &valp->toObject(); - traverse(&obj); - valp->setObject(*obj); + *thingp = DispatchTyped(TenuringTraversalFunctor(), *thingp, this); } - -template <> -void -TenuringTracer::traverse(TaggedProto* protop) -{ - if (!protop->isObject()) - return; - - JSObject *obj = protop->toObject(); - traverse(&obj); - *protop = TaggedProto(obj); -} - -template <> void js::TenuringTracer::traverse(js::BaseShape**) {} -template <> void js::TenuringTracer::traverse(js::jit::JitCode**) {} -template <> void js::TenuringTracer::traverse(JSScript**) {} -template <> void js::TenuringTracer::traverse(js::LazyScript**) {} -template <> void js::TenuringTracer::traverse(js::Shape**) {} -template <> void js::TenuringTracer::traverse(JSString**) {} -template <> void js::TenuringTracer::traverse(JS::Symbol**) {} -template <> void js::TenuringTracer::traverse(js::ObjectGroup**) {} -template <> void js::TenuringTracer::traverse(jsid*) {} } // namespace js template @@ -2310,13 +2259,13 @@ IsMarkedInternalCommon(T* thingp) template static bool -IsMarkedInternal(T* thingp) +IsMarkedInternal(T** thingp) { return IsMarkedInternalCommon(thingp); } -template -static bool +template <> +/* static */ bool IsMarkedInternal(JSObject** thingp) { if (IsInsideNursery(*thingp)) { @@ -2335,39 +2284,21 @@ struct IsMarkedFunctor : public IdentityDefaultAdaptor { } }; -template <> -bool -IsMarkedInternal(Value* valuep) +template +static bool +IsMarkedInternal(T* thingp) { bool rv = true; - *valuep = DispatchValueTyped(IsMarkedFunctor(), *valuep, &rv); - return rv; -} - -template <> -bool -IsMarkedInternal(jsid* idp) -{ - bool rv = true; - *idp = DispatchIdTyped(IsMarkedFunctor(), *idp, &rv); - return rv; -} - -template <> -bool -IsMarkedInternal(TaggedProto* protop) -{ - bool rv = true; - *protop = DispatchTaggedProtoTyped(IsMarkedFunctor(), *protop, &rv); + *thingp = DispatchTyped(IsMarkedFunctor(), *thingp, &rv); return rv; } template static bool -IsAboutToBeFinalizedInternal(T* thingp) +IsAboutToBeFinalizedInternal(T** thingp) { CheckIsMarkedThing(thingp); - T thing = *thingp; + T* thing = *thingp; JSRuntime* rt = thing->runtimeFromAnyThread(); /* Permanent atoms are never finalized by non-owning runtimes. */ @@ -2404,30 +2335,12 @@ struct IsAboutToBeFinalizedFunctor : public IdentityDefaultAdaptor { } }; -template <> -bool -IsAboutToBeFinalizedInternal(Value* valuep) +template +static bool +IsAboutToBeFinalizedInternal(T* thingp) { bool rv = false; - *valuep = DispatchValueTyped(IsAboutToBeFinalizedFunctor(), *valuep, &rv); - return rv; -} - -template <> -bool -IsAboutToBeFinalizedInternal(jsid* idp) -{ - bool rv = false; - *idp = DispatchIdTyped(IsAboutToBeFinalizedFunctor(), *idp, &rv); - return rv; -} - -template <> -bool -IsAboutToBeFinalizedInternal(TaggedProto* protop) -{ - bool rv = false; - *protop = DispatchTaggedProtoTyped(IsAboutToBeFinalizedFunctor(), *protop, &rv); + *thingp = DispatchTyped(IsAboutToBeFinalizedFunctor(), *thingp, &rv); return rv; } diff --git a/js/src/gc/Marking.h b/js/src/gc/Marking.h index 8c190d61988..dcc042234d2 100644 --- a/js/src/gc/Marking.h +++ b/js/src/gc/Marking.h @@ -183,10 +183,8 @@ class GCMarker : public JSTracer template void traverse(T thing); // Calls traverse on target after making additional assertions. + template void traverseEdge(S source, T* target); template void traverseEdge(S source, T target); - // C++ requires explicit declarations of partial template instantiations. - template void traverseEdge(S source, jsid target); - template void traverseEdge(S source, Value target); /* * Care must be taken changing the mark color from gray to black. The cycle @@ -471,6 +469,10 @@ DECLARE_REWRAP(js::TaggedProto, JSObject*, js::TaggedProto, ); bool UnmarkGrayShapeRecursively(Shape* shape); +template +void +CheckTracedThing(JSTracer* trc, T* thing); + template void CheckTracedThing(JSTracer* trc, T thing); diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index 7c4b4070ddb..267578f25c1 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -66,6 +66,7 @@ class TenuringTracer : public JSTracer const Nursery& nursery() const { return nursery_; } // Returns true if the pointer was updated. + template void traverse(T** thingp); template void traverse(T* thingp); void insertIntoFixupList(gc::RelocationOverlay* entry); diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index ea326b4ab45..7472b2e075d 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -413,7 +413,7 @@ BufferGrayRootsTracer::onChild(const JS::GCCellPtr& thing) // objects and scripts. We rely on gray root buffering for this to work, // but we only need to worry about uncollected dead compartments during // incremental GCs (when we do gray root buffering). - DispatchTraceKindTyped(SetMaybeAliveFunctor(), tenured, thing.kind()); + DispatchTyped(SetMaybeAliveFunctor(), thing); if (!zone->gcGrayRoots.append(tenured)) bufferingGrayRootsFailed = true; diff --git a/js/src/gc/Tracer.cpp b/js/src/gc/Tracer.cpp index a644650e3c5..3732767f885 100644 --- a/js/src/gc/Tracer.cpp +++ b/js/src/gc/Tracer.cpp @@ -67,7 +67,7 @@ template <> Value DoCallback(JS::CallbackTracer* trc, Value* vp, const char* name) { - *vp = DispatchValueTyped(DoCallbackFunctor(), *vp, trc, name); + *vp = DispatchTyped(DoCallbackFunctor(), *vp, trc, name); return *vp; } @@ -75,7 +75,7 @@ template <> jsid DoCallback(JS::CallbackTracer* trc, jsid* idp, const char* name) { - *idp = DispatchIdTyped(DoCallbackFunctor(), *idp, trc, name); + *idp = DispatchTyped(DoCallbackFunctor(), *idp, trc, name); return *idp; } @@ -83,7 +83,7 @@ template <> TaggedProto DoCallback(JS::CallbackTracer* trc, TaggedProto* protop, const char* name) { - *protop = DispatchTaggedProtoTyped(DoCallbackFunctor(), *protop, trc, name); + *protop = DispatchTyped(DoCallbackFunctor(), *protop, trc, name); return *protop; } diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index f945eb60bf6..2d785f4141a 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -3763,7 +3763,7 @@ CompartmentCheckTracer::onChild(const JS::GCCellPtr& thing) { TenuredCell* tenured = TenuredCell::fromPointer(thing.asCell()); - JSCompartment* comp = DispatchTraceKindTyped(MaybeCompartmentFunctor(), tenured, thing.kind()); + JSCompartment* comp = DispatchTyped(MaybeCompartmentFunctor(), thing); if (comp && compartment) { MOZ_ASSERT(comp == compartment || runtime()->isAtomsCompartment(comp) || (srcKind == JS::TraceKind::Object && @@ -7332,7 +7332,7 @@ JS::IncrementalReferenceBarrier(GCCellPtr thing) if (!thing) return; - DispatchTraceKindTyped(IncrementalReferenceBarrierFunctor(), thing.asCell(), thing.kind()); + DispatchTyped(IncrementalReferenceBarrierFunctor(), thing); } JS_PUBLIC_API(void) diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 7115d84bebb..40c3cb72490 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -1189,7 +1189,7 @@ struct IsForwardedFunctor : public BoolDefaultAdaptor { inline bool IsForwarded(const JS::Value& value) { - return DispatchValueTyped(IsForwardedFunctor(), value); + return DispatchTyped(IsForwardedFunctor(), value); } template @@ -1210,7 +1210,7 @@ struct ForwardedFunctor : public IdentityDefaultAdaptor { inline Value Forwarded(const JS::Value& value) { - return DispatchValueTyped(ForwardedFunctor(), value); + return DispatchTyped(ForwardedFunctor(), value); } template @@ -1246,7 +1246,7 @@ struct CheckValueAfterMovingGCFunctor : public VoidDefaultAdaptor { inline void CheckValueAfterMovingGC(const JS::Value& value) { - DispatchValueTyped(CheckValueAfterMovingGCFunctor(), value); + DispatchTyped(CheckValueAfterMovingGCFunctor(), value); } #endif // JSGC_HASH_TABLE_CHECKS diff --git a/js/src/vm/TaggedProto.h b/js/src/vm/TaggedProto.h index c20d833ea79..72538802535 100644 --- a/js/src/vm/TaggedProto.h +++ b/js/src/vm/TaggedProto.h @@ -104,7 +104,7 @@ class BarrieredBaseMixins : public TaggedProtoOperations auto -DispatchTaggedProtoTyped(F f, TaggedProto& proto, Args&&... args) +DispatchTyped(F f, TaggedProto& proto, Args&&... args) -> decltype(f(static_cast(nullptr), mozilla::Forward(args)...)) { if (proto.isObject()) diff --git a/js/src/vm/UbiNode.cpp b/js/src/vm/UbiNode.cpp index 8c08d3d4bda..f1badf95771 100644 --- a/js/src/vm/UbiNode.cpp +++ b/js/src/vm/UbiNode.cpp @@ -37,7 +37,7 @@ using mozilla::Some; using mozilla::RangedPtr; using mozilla::UniquePtr; -using JS::DispatchTraceKindTyped; +using JS::DispatchTyped; using JS::HandleValue; using JS::Value; using JS::ZoneSet; @@ -166,12 +166,12 @@ struct Node::ConstructFunctor : public js::BoolDefaultAdaptor { Node::Node(const JS::GCCellPtr &thing) { - DispatchTraceKindTyped(ConstructFunctor(), thing.asCell(), thing.kind(), this); + DispatchTyped(ConstructFunctor(), thing, this); } Node::Node(HandleValue value) { - if (!DispatchValueTyped(ConstructFunctor(), value, this)) + if (!DispatchTyped(ConstructFunctor(), value, this)) construct(nullptr); }