Closed Bug 1730993 Opened 3 years ago Closed 3 years ago

Raise the priority of RequestContentRepaint call

Categories

(Core :: Panning and Zooming, enhancement)

enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1730998

People

(Reporter: hiro, Unassigned)

References

Details

this.

The RequestContentRepaint call basically happens when we detect jank in APZ, if the user keeps scrolling fast (by scrollbar drag or some such), we do quite often jank in APZ, thus we send a bunch of RequestContentRepaints to the content process, in the mean time if the main-thread in the content process is busy on some tasks, the RequestContentRepaints are not processed effectively, whereas on the compositor thread APZ can keep sending new RequestContentRepaints, it will result long jank unfortunately.

This is actually what happens in bug 1585378. So I'd like to raise the priority of the RequestContentRepaint call. In the test case in bug 1585378 using [Priority=input] mostly eliminates flickers on my Linux desktop (it's a Ryzen 5950x machine so it's super fast though), on my Windows laptop (lenovo Thinkpad 460p) it mitigates the flicker to some extent.

Jeff told me on Matrix it sounds reasonable. Olli, what do you think about it?

Olli ^

(I've been wondering why bugzilla doesn't allow us to leave NI when a new bug is opened)

Flags: needinfo?(bugs)

Priority=input would be odd. Input has its own rather magical handling, where we try to process input tasks right before the next vsync.
But if there is no pending vsync, it has lower priority. And if handling of input tasks take too much time, vsync can run.

I'd rather use Priority=mediumhigh.
Or, one more effective option could be to pass the data to the child process using a Priority=control message, but it would just
create a RefreshDriver Early Runner (https://searchfox.org/mozilla-central/source/layout/base/nsRefreshDriver.h#210-218) which would then trigger the stuff RecvRequestContentRepaint currently does.
That way a new paint would be scheduled asap, and RequestContentRepaint would be processed right at the beginning of the RefreshDriver tick and possibly extra reflows (caused by scrolling) were handled during tick time.
And scroll event would still occur after the RequestContentRepaint handling
https://searchfox.org/mozilla-central/rev/4d90ff4537330d6b17cb956c0fadf759086d9bb7/layout/base/nsRefreshDriver.cpp#2267,2269,2343

If we want to just paint asap (not do reflows or such), it might be possible to reuse the code from tab switching. JS interrupt callback checks whether there is need for a paint and if so, paints synchronously (but doesn't run a full refreshdriver tick).
https://searchfox.org/mozilla-central/rev/4d90ff4537330d6b17cb956c0fadf759086d9bb7/dom/ipc/ProcessHangMonitor.cpp#336,364
This setup helps obviously only with long running JS scripts.
But looks like RequestContentRepaint does more. It wants to also scroll, which means doing possibly reflows, so maybe it can't reuse that setup.

Flags: needinfo?(bugs)

This will be superseded by bug 1730998.
Thank you Olli, I'd take the way you suggested here.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.