Closed Bug 1552401 Opened 7 months ago Closed 7 months ago

When decommit runs on the main thread it is accounted in the compact phase.

Categories

(Core :: JavaScript: GC, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: pbone, Assigned: pbone)

References

Details

Attachments

(2 files, 1 obsolete file)

If a decommit thread should not be spawned or fails to spawn, then decommit is done in the foreground, but it is done in the compact phase and therefore reported incorrectly.

(In reply to Paul Bone [:pbone] from comment #0)

Oh yes, I see, this does happen while the incremental state is State::Compact. But how does it get reported incorrectly? If it happens in the foreground it's not incremental and this shouldn't be observable.

Or is this on top of your incremental decommit changes?

Flags: needinfo?(pbone)

No, this is just on mozilla-central, no other changes.

It's only observable in things like the gecko profiler and aother things that report how much time is spent in each phase. It doesn't affect users at all.

Flags: needinfo?(pbone)

(In reply to Paul Bone [:pbone] from comment #3)

I don't understand how the gecko profiler gets this information either.

(In reply to Jon Coppeard (:jonco) from comment #4)

I don't understand how the gecko profiler gets this information either.

Through the use of the AutoPhase class in each step of incrementalSlice. It calls beginPhase() and endPhase() on the statistics object which records them. Later when generating the data this data is retrieved from the Statistics object and written out in JSON. The profiler readers some of the phase information as part of the marker tooltip.

(In reply to Paul Bone [:pbone] from comment #5)

Through the use of the AutoPhase class in each step of incrementalSlice.

Ah OK, but there's isn't an AutoPhase in use in the compact case in incrementalSlice. And the decommit case uses PhaseKind::WAIT_BACKGROUND_THREAD, which isn't what we want either.

Maybe you could add an AutoPhase to the startDecommit method instead? You'll need to add a stats phase in GeneratePhases.py.

True. That's a simpler solution. I may use this code later to incrementalise on-main-thread decommit.

Attachment #9065607 - Attachment is obsolete: true
Pushed by pbone@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ca8cc8b10cef
Make GenerateStatsPhases.py print the next available bucket number r=jonco
https://hg.mozilla.org/integration/autoland/rev/8ce4b2a4216c
Add and use a new decommit phase to attribute decommit work r=jonco
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Regressions: 1555101
You need to log in before you can comment on or make changes to this bug.