Closed Bug 1360946 Opened 3 years ago Closed 3 years ago
Interruptible layout does not appear to be interrupted by force painting with e10s
Bug 1360946 - Consider a SetDocShellIsActive message a 'pending input event' so it can interrupt reflow.
59 bytes, text/x-review-board-request
I _think_ I understand how and where interruptible reflows happen when using Gecko. My (perhaps low resolution) understanding is that these are layout calculations that only occur on initial page load, and that things like input events or maybe event a timeout can result in the reflow being interrupted and continued later. I _suspect_ that tab switching is one of those things that isn't interrupting these interruptible reflows. I captured a particularly long interruptible reflow in the profiler. I was loading a tryserver log in the background, and then immediately switched back to a treeherder tab. I've highlighted the point where the content process (4 of 4) received the message to force paint: https://perf-html.io/public/2135d14a1a93d86f407c56ad854092af412c2846/calltree/?range=58225.6535_59213.8689~59106.9992_59213.8689~59201.0060_59203.1183&thread=7 But here's the point where I requested the tab - right at the start of the reflow: https://perf-html.io/public/07530fa654c4fbd641c9766894cd69a48e90c775/calltree/?callTreeFilters=prefix-0123456789zN6zXeQjFLpyHqxOpxwrCF3NpNqNrNsDK2O0R2O2O3CykDX2xU3zMazLdxC9BQuxCbAAnylx0GRhx2xU4xU5FEczI2xUmz3GRhx2xUeFEczI2xUmz3GRhx2xUfFEczI2xUmz3GRhx2xUgFEczI2xUmz3GRhx2xUk&thread=0 So the process hang monitor thread had to wait quite a while before it could cause the force paint. Rounding down aggressively, the tab switch request happened in the parent process around the 6s mark in the profile. The force paint occurred around 14s. That means I had to wait 8s for the force paint to occur. In any case, I wonder if perhaps the JS interruption stuff that we do for tab switching can / should be extended to interrupt interruptible reflow as well. Or do we already do that, and I'm misunderstanding this profile? : I was doing a build in the background, so my machine was under pretty heavy load.
Does the above sound plausible, or am I off my rocker?
Well, interruptible reflow means that we stop reflow at a specific interruption point and return back to the event loop. For it to work, we need to be able to dirty the frames that haven't been reflowed yet so that we can reflow them correctly the next time we reflow. The stuff we do for interrupting JS is a bit different. We do the painting while the JS is still on the stack. We don't need to worry about dirtying stuff since we can just proceed where we left off when painting finishes. That, in theory, makes it easier to interrupt to paint: we don't need to save our state. However, there are a couple problems: First, you need to choose a good interruption point. Based on the profile, we spent 4 seconds in SplitAndInitTextRun. For your optimization to be useful, you would need to be able to interrupt that code. Honestly, I doubt we would ever spend so much time in this function if we weren't displaying an enormous text file (as you are). So I'm not sure that this is that realistic a scenario. In any case, I don't know much about that code, so I don't know how easy it would be to interrupt there. In general, if you interrupt somewhere, you have to make sure that whatever state things are left in is a good state to paint in. I ran into this problem in https://bugzilla.mozilla.org/show_bug.cgi?id=1305130#c7. I suspect you may hit similar issues with what you're trying to do. Anyway, reading that bug might give you some insight into the difficulties you might see here.
Thinking about this a bit more, it's a lot easier to interrupt a TabGroup different than the one you intend to paint. It looks like that's the situation here. So this should be relatively easy to fix. It's also possible that this would naturally be fixed by Quantum DOM work.
Ooooh boy this is pretty speculative. Basically, from what I can tell, HasPendingInputEvent checks the message queue for a TabChild periodically during layout to see if there are any incoming messages for user input - and if so, I think in some cases we can interrupt layout to paint. This patch adds mozilla::dom::PBrowser::Msg_SetDocShellIsActive__ID to the list of message IDs that it will interrupt layout for. What this means is that if we've started layout in a background tab, and the user then selects that background tab, presumably, we'll have the opportunity to paint something sooner rather than later. Interruptible reflow isn't something I super understand right now, but I thought I'd give this a shot.
I am not sure if I am suitable to review the patch, but I have a few questions: 1) why do we layout a background tab before it is active, is it for pre-render? 2) when do we send SetDocShellIsActive? do we send it only when the tab is moving to foreground? 3) what if we make force paint the interruption point instead?
Flags: needinfo?(mconley) → needinfo?(janus926)
:bz, could you review this?
Comment on attachment 8871968 [details] Bug 1360946 - Consider a SetDocShellIsActive message a 'pending input event' so it can interrupt reflow. https://reviewboard.mozilla.org/r/143500/#review151092 r=me
Attachment #8871968 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/73eefb81f8e1 Consider a SetDocShellIsActive message a 'pending input event' so it can interrupt reflow. r=bz
Assignee: nobody → mconley
Iteration: --- → 55.7 - Jun 12
Priority: -- → P1
You need to log in before you can comment on or make changes to this bug.