Closed Bug 1306728 Opened 8 years ago Closed 8 years ago

Allow painting for tab switch when content process is running the cycle collector

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox52 --- affected

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file, 1 obsolete file)

Mostly the cycle collector is very fast, with pauses less than 5ms, but when cleaning up after page closing it can sometimes take a second or more to finish. Being able to interrupt the cycle collector for higher priority work was Andreas Gal's long-running dream (bug 565217), and he asks me about it every time he sees me. In bug 1279086, Bill added a mechanism where the main process can send the child process a message (off the main thread), telling it to paint immediately, and then added code to the JS interrupt callback to see if it should paint immediately, due to tab switching, and then triggering a paint through some means I don't understand. Bill mentioned that it would be nice to have this for GC and CC, given that they are major sources of long pauses in the content process. Now that we have incremental CC (bug 850065), it shouldn't be too hard to use the same mechanism to interrupt CC graph building (and maybe at some other points), and then trigger a paint. This can trigger random DOM code running, but ICC should be able to deal with that anyways. I have a prototype patch that seems to work with at least basic testing.
There are two tricky parts (at least, that I've noticed so far): 1. The code that painting runs while the CC is interrupted must be treated like any regular non-CC code, so my patch sets mScanInProgress to false while it is running, and also does not assert if there are any GCs. 2. A CC that runs in a single slice must still be treated as an "incremental CC" if it was interrupted at any point during graph building. This will trigger the ICC fixup code in ScanIncrementalRoots().
Attached patch Prototype of interruptible CC. (obsolete) — Splinter Review
This prototype introduces an intentional 5 second hang in every slice of graph build to make it easier to see what effect it has. The actual version of this patch to land would be slightly different. MozReview-Commit-ID: Dka0OFGPTEs
This new version is simpler. Instead of worrying about inverting the state here and there inside BuildGraph(), it instead just returns to the CC::Collect top loop and does it there.
Attachment #8796709 - Attachment is obsolete: true
See Also: → 1308039
The GC version of this didn't show any improvements on telemetry, so I'm going to assume this won't help either, until we have better evidence that it will.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: