Closed Bug 1384885 Opened 2 years ago Closed 2 years ago

Time spent gray unmarking cross-zone edges is not accounted for


(Core :: JavaScript: GC, enhancement)

55 Branch
Not set



Tracking Status
firefox56 --- fixed


(Reporter: jonco, Assigned: jonco)



(1 file)

When we find a cross-zone black to gray edge during marking, we put the target in a buffer and do gray unmarking on it later using AutoExposeLiveCrossZoneEdges.  This happens outside any statistics phase though so this is not included in our telemetry.  Also it happens after we've checked our slice budget so can cause our slices to overrun their budget.
Rather than buffering gray cells to unmark, this patch unmarks them immediately.

I had to add the UNMARK_GRAY phase under all stats phases that can do black marking.  I changed Statistics::lookupChildPhase() because I found that if you try and start a phase in the wrong parent you will enter an infinte loop in release builds (and even in debug builds you only get an unhelpful assertion failure).
Attachment #8890823 - Flags: review?(sphink)
Comment on attachment 8890823 [details] [diff] [review]

Review of attachment 8890823 [details] [diff] [review]:

::: js/src/gc/
@@ +70,5 @@
>  ])
>  JoinParallelTasksPhaseKind = PhaseKind("JOIN_PARALLEL_TASKS", "Join Parallel Tasks", 67)
> +UnmarkGrayPhaseKind = PhaseKind("UNMARK_GRAY", "Unmark gray", 56)

Hm. I wonder if this would be a good place to comment something like

 # Whenever we mark a cross-compartment wrapper, we might need to unmark gray on the destination to avoid a black->gray edge.

? (At least, I *think* that's why adding all these in is necessary.)
Attachment #8890823 - Flags: review?(sphink) → review+
Pushed by
Do gray unmarking of cross zone edges as soon as they are found r=sfink
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.