1144331 - Assert that gray tracing does not depend on being a marking tracer

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

Attachment: 2.1_make_ismarkingtracer_more_accurate-v0.diff

I'm having trouble stabilizing bug 1135985 patch #2 -- misc impossible-to-debug watchdog timeouts in CC tracing during shutdown that only repro on infra in winxp debug builds.

Thus, I'm splitting that patch up and testing it incrementally. This piece adds a single assertion (plus some long-desired refactoring): crash if we check if a thing is a marking tracer while buffering gray roots. The idea here is that we're changing gray buffering from a marking tracer to a callback tracer, so we'd potentially start marking a different set. By inspection I can't see a way we could reach this: this patch just asserts that I am correct.

Try run at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0be4cb46b140

Comment 1

[Jon Coppeard (:jonco)]

Comment on attachment 8578870 [details] [diff] [review]
2.1_make_ismarkingtracer_more_accurate-v0.diff

Review of attachment 8578870 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

::: js/src/gc/Tracer.h
@@ +353,5 @@
>  void
>  SetMarkStackLimit(JSRuntime *rt, size_t limit);
>  
> +// Return true if this trace is happening on behalf of the the marking phase
> +// of the GC.

Comment needs updating to mention gray marking.  And to remove the second 'the'.

@@ +360,5 @@
> +{
> +    return trc->callback == js::GCMarker::GrayCallback;
> +}
> +
> +// Return true if this trace is happening on behalf of the the marking phase

Ditto 'the the'.

Comment 2

[Jon Coppeard (:jonco)]

The try build seems to have hit a load of infra issues, so I pushed another one:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4085e5abd78

Comment 3

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13c8a1cc5ed3

Comment 4

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/701f405c730d

Comment 5

[Terrence Cole [:terrence]]

D'oh! Clicked on the wrong tab when sorting these checkins into bugs, ignore that last comment.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13c8a1cc5ed3