From d54dda2c97794c3c33cbddd33458e117cf121210 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Mon, 19 Aug 2013 14:48:35 +0100 Subject: [PATCH] Bug 903548 - GC: What do we do for UnmarkGray on a Nursery GCThing? r=billm --- js/public/GCAPI.h | 2 +- js/public/HeapAPI.h | 10 ++++++ js/src/gc/Marking.cpp | 48 ++++++++++++++++++-------- js/src/jsfriendapi.cpp | 3 +- xpcom/base/CycleCollectedJSRuntime.cpp | 10 +++--- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index e7b4d809a568..9c2dc433f4ae 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -256,7 +256,7 @@ class ObjectPtr * Unsets the gray bit for anything reachable from |thing|. |kind| should not be * JSTRACE_SHAPE. |thing| should be non-null. */ -extern JS_FRIEND_API(void) +extern JS_FRIEND_API(bool) UnmarkGrayGCThingRecursively(void *thing, JSGCTraceKind kind); /* diff --git a/js/public/HeapAPI.h b/js/public/HeapAPI.h index 4d739304bcb3..ec83caf9e8bd 100644 --- a/js/public/HeapAPI.h +++ b/js/public/HeapAPI.h @@ -128,6 +128,16 @@ GetObjectZone(JSObject *obj) static JS_ALWAYS_INLINE bool GCThingIsMarkedGray(void *thing) { +#ifdef JSGC_GENERATIONAL + /* + * GC things residing in the nursery cannot be gray: they have no mark bits. + * All live objects in the nursery are moved to tenured at the beginning of + * each GC slice, so the gray marker never sees nursery things. + */ + JS::shadow::Runtime *rt = js::gc::GetGCThingRuntime(thing); + if (uintptr_t(thing) >= rt->gcNurseryStart_ && uintptr_t(thing) < rt->gcNurseryEnd_) + return false; +#endif uintptr_t *word, mask; js::gc::GetGCThingMarkWordAndMask(thing, js::gc::GRAY, &word, &mask); return *word & mask; diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index efa1cf394003..f86947281cd9 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -651,10 +651,15 @@ ShouldMarkCrossCompartment(JSTracer *trc, JSObject *src, Cell *cell) if (!IS_GC_MARKING_TRACER(trc)) return true; - JS::Zone *zone = cell->tenuredZone(); uint32_t color = AsGCMarker(trc)->getMarkColor(); - JS_ASSERT(color == BLACK || color == GRAY); + + if (IsInsideNursery(trc->runtime, cell)) { + JS_ASSERT(color == BLACK); + return false; + } + + JS::Zone *zone = cell->tenuredZone(); if (color == BLACK) { /* * Having black->gray edges violates our promise to the cycle @@ -1578,14 +1583,14 @@ struct UnmarkGrayTracer : public JSTracer * up any color mismatches involving weakmaps when it runs. */ UnmarkGrayTracer(JSRuntime *rt) - : tracingShape(false), previousShape(NULL) + : tracingShape(false), previousShape(NULL), unmarkedAny(false) { JS_TracerInit(this, rt, UnmarkGrayChildren); eagerlyTraceWeakMaps = DoNotTraceWeakMaps; } UnmarkGrayTracer(JSTracer *trc, bool tracingShape) - : tracingShape(tracingShape), previousShape(NULL) + : tracingShape(tracingShape), previousShape(NULL), unmarkedAny(false) { JS_TracerInit(this, trc->runtime, UnmarkGrayChildren); eagerlyTraceWeakMaps = DoNotTraceWeakMaps; @@ -1596,6 +1601,9 @@ struct UnmarkGrayTracer : public JSTracer /* If tracingShape, shape child or NULL. Otherwise, NULL. */ void *previousShape; + + /* Whether we unmarked anything. */ + bool unmarkedAny; }; /* @@ -1642,10 +1650,14 @@ UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind) return; } - if (!JS::GCThingIsMarkedGray(thing)) - return; + UnmarkGrayTracer *tracer = static_cast(trc); + if (!IsInsideNursery(trc->runtime, thing)) { + if (!JS::GCThingIsMarkedGray(thing)) + return; - UnmarkGrayGCThing(thing); + UnmarkGrayGCThing(thing); + tracer->unmarkedAny = true; + } /* * Trace children of |thing|. If |thing| and its parent are both shapes, @@ -1654,12 +1666,12 @@ UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind) * depth during shape tracing. It is safe to do because a shape can only * have one child that is a shape. */ - UnmarkGrayTracer *tracer = static_cast(trc); UnmarkGrayTracer childTracer(tracer, kind == JSTRACE_SHAPE); if (kind != JSTRACE_SHAPE) { JS_TraceChildren(&childTracer, thing, kind); JS_ASSERT(!childTracer.previousShape); + tracer->unmarkedAny |= childTracer.unmarkedAny; return; } @@ -1675,19 +1687,27 @@ UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind) thing = childTracer.previousShape; childTracer.previousShape = NULL; } while (thing); + tracer->unmarkedAny |= childTracer.unmarkedAny; } -JS_FRIEND_API(void) +JS_FRIEND_API(bool) JS::UnmarkGrayGCThingRecursively(void *thing, JSGCTraceKind kind) { JS_ASSERT(kind != JSTRACE_SHAPE); - if (!JS::GCThingIsMarkedGray(thing)) - return; - - UnmarkGrayGCThing(thing); - JSRuntime *rt = static_cast(thing)->runtimeFromMainThread(); + + bool unmarkedArg = false; + if (!IsInsideNursery(rt, thing)) { + if (!JS::GCThingIsMarkedGray(thing)) + return false; + + UnmarkGrayGCThing(thing); + unmarkedArg = true; + } + UnmarkGrayTracer trc(rt); JS_TraceChildren(&trc, thing, kind); + + return unmarkedArg || trc.unmarkedAny; } diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 622537f941b5..3e550c56fb03 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -591,10 +591,11 @@ js::GCThingTraceKind(void *thing) JS_FRIEND_API(void) js::VisitGrayWrapperTargets(Zone *zone, GCThingCallback callback, void *closure) { + JSRuntime *rt = zone->runtimeFromMainThread(); for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) { for (JSCompartment::WrapperEnum e(comp); !e.empty(); e.popFront()) { gc::Cell *thing = e.front().key.wrapped; - if (thing->isMarked(gc::GRAY)) + if (!IsInsideNursery(rt, thing) && thing->isMarked(gc::GRAY)) callback(closure, thing); } } diff --git a/xpcom/base/CycleCollectedJSRuntime.cpp b/xpcom/base/CycleCollectedJSRuntime.cpp index 30b476f061ac..ecb85883e958 100644 --- a/xpcom/base/CycleCollectedJSRuntime.cpp +++ b/xpcom/base/CycleCollectedJSRuntime.cpp @@ -258,8 +258,9 @@ private: if (delegateMightNeedMarking && kkind == JSTRACE_OBJECT) { JSObject* kdelegate = js::GetWeakmapKeyDelegate((JSObject*)k); if (kdelegate && !xpc_IsGrayGCThing(kdelegate)) { - JS::UnmarkGrayGCThingRecursively(k, JSTRACE_OBJECT); - tracer->mAnyMarked = true; + if (JS::UnmarkGrayGCThingRecursively(k, JSTRACE_OBJECT)) { + tracer->mAnyMarked = true; + } } } @@ -267,8 +268,9 @@ private: (!k || !xpc_IsGrayGCThing(k)) && (!m || !xpc_IsGrayGCThing(m)) && vkind != JSTRACE_SHAPE) { - JS::UnmarkGrayGCThingRecursively(v, vkind); - tracer->mAnyMarked = true; + if (JS::UnmarkGrayGCThingRecursively(v, vkind)) { + tracer->mAnyMarked = true; + } } }