1257869 - Window Resize of large page takes seconds to change viewport

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Chris H-C :chutten]

Attachment: viewportResizeScreenshot.png

Nightly 48, 2016-03-17, Windows 10 x64

STR:
1. open https://hg.mozilla.org/mozilla-central/rev/341344bdec8f10bf50646cd6ef2355361435cbf6 
2. Resize the window by mouse-dragging the bottom-right corner down and to the right

Expected: prompt resizing of contents

Actual: Six second delay, then contents resize.

Not sure what details are important here. It doesn't happen on just any page. I don't know other pages quite as wide and tall as this one to test to see if the delay is proportional to the page dimensions.

Observation: While resizing, the viewport jitters about as though it's resizing to within a rounding error of where it is and then is being snapped back. Maybe the destination rect isn't being updated during the resize operation?

Comment 1

[Jonathan Kew [:jfkthame]]

Is this e10s-specific, or does it reproduce with that disabled?

Comment 2

[Chris H-C :chutten]

e10s-specific. Forgot to mention. On !e10s the resize is slow as molasses, but the viewport size is within a frame or two of the window size.

Trying to find a regression range, but maybe this page is just pathalogical. It still sits there without resizing at least as far back as 2016-01-10, but at least back then it resized within about 2 seconds or immediately upon trying to scroll.

I'll keep digging backwards.

Comment 3

[Jonathan Kew [:jfkthame]]

I see similar behavior on OS X with that page, it turns out. So it's not Win32-specific.

This may not be a widget issue in the end (sorry, I'd misunderstood the symptoms a bit), but I'm not sure where it does belong. Definitely makes e10s look bad, though.

Comment 4

[Chris H-C :chutten]

The page isn't handled terribly well on !e10s, for what it's worth. But at least !e10s seems like it's _trying_ to keep up.

Comment 5

[Chris H-C :chutten]

Goodbye "regression" flag. I can't actually find a nightly build that doesn't exhibit these symptoms with e10s enabled.

So, uh, whoever's interested in resize performance: have I found a pathalogical case for you!

Comment 6

[Chris H-C :chutten]

Attachment: chromeIsntHappyWithItEither.PNG

To be fair to Firefox and E10s, Chrome doesn't handle it very adroitly either...

Comment 7

[Jim Mathies [:jimm]]

non-e10s definitely handles this better. I also tried without apz, things improved a bit but there is still noticeable lag.

Comment 8

[Ting-Yu Chou [:ting] (away)]

This is actually a duplicate of bug 1255968, which mozilla::dom::PBrowser::Msg_UpdateDimensions__ID also needs to be checked in PuppetWidget::HasPendingInputEvent() to interrupt the reflow.

Comment 9

[Ting-Yu Chou [:ting] (away)]

Alice, could you also verify whether this issue is fixed by bug 1255968? Thank you.

Comment 10

[Alice0775 White]

e10s is till x2 slower than non-e10s.

Build[1](before landing Bug 1255968):
It takes about 10 sec when resized browser window 600*600 -> 1280*1000
It takes about 10 sec when resized browser window 1280*1000 -> 600*600

Build[1](before landing Bug 1255968) with disabled e10s:
It takes about 5 sec when resized browser window 600*600 -> 1280*1000
It takes about 5 sec when resized browser window 1280*1000 -> 600*600


Build[2](after landing Bug 1255968):
1st try
It takes about 10 sec when resized browser window 600*600 -> 1280*1000
It takes about 10 sec when resized browser window 1280*1000 -> 600*600

2nd and 3rd trys
It takes about 10 sec when resized browser window 600*600 -> 1280*1000
It takes about 6 sec when resized browser window 1280*1000 -> 600*600
It takes about 10 sec when resized browser window 600*600 -> 1280*1000
It takes about 6 sec when resized browser window 1280*1000 -> 600*600

Build[2](after landing Bug 1255968) with disabled e10s:
It takes about 5 sec when resized browser window 600*600 -> 1280*1000
It takes about 5 sec when resized browser window 1280*1000 -> 600*600


*I mesure the time when restore function of scrollbars and mouse wheel scroll after resize window

[1]https://hg.mozilla.org/mozilla-central/rev/494289c72ba3997183e7b5beaca3e0447ecaf96d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 ID:20160331030231

[2]https://hg.mozilla.org/mozilla-central/rev/538d248fa252a4100082fd9bc3fdc08d322cda22
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 ID:20160401030216

Comment 11

[Ting-Yu Chou [:ting] (away)]

(In reply to Alice0775 White from comment #10)
> *I mesure the time when restore function of scrollbars and mouse wheel
> scroll after resize window

Not sure is this matched to what the issue expects, Chris?

Per my local test, the content is resized promptly, but it takes a while for the scrollbar to response if you try to drag it right after resize, use mouse wheel to scroll is fine though. I guess that means some other events also need to be monitored by PuppetWidget::HasPendingInputEvent

Comment 12

[Chris H-C :chutten]

Nightly 2016-04-06:

The problem still occurs. The scrollbars lag significantly behind the resize. It appears to be better, and in some cases the scrollbars almost immediately resize to geometry that is really _close_ to the correct size before the stall happens, then they snap to the current size.

Comment 13

[Ting-Yu Chou [:ting] (away)]

The reflow is not interrupted as frequent as non-e10s, I'll try to figure out how are the mouse events collected and passed to content process because nsWindow::HasPendingInputEvent() calls native API to check, but PuppetWidget::HasPendingInputEvent() relies on IPC and there seems to be some delay.

Comment 14

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #13)
> PuppetWidget::HasPendingInputEvent() relies on IPC and there seems to be
> some delay.

It's not about delay.

UpdateDimensions() is a "compressall" IPC call, which there can be at most one message in the queue even though parent sends many. And there're no other (mouse) messages will interrupt the reflow while handling the last UpdateDimensions message.

Removing "compressall" makes the scrollbars lag worse, I don't know the relation between reflow and how/when scrollbar's position is updated and flushed to the screen yet, will try to figure it out. Also I wonder whether scrollbar updating on e10s is different from non-e10s.

Comment 15

[Ting-Yu Chou [:ting] (away)]

Profiles:
  non-e10s: https://cleopatra.io/#report=f34cfc208d26f5e27aac5cd9b780152cb20ca955
  e10s:     https://cleopatra.io/#report=796aec9e167544ee3b5bef6c222f459e52c9bce8

Comment 16

[Ting-Yu Chou [:ting] (away)]

Is it possible to interrupt a long reflow whenever all the elements in current viewport have been reflowed, so user can see updated content even though the full page reflow hasn't been finished?

Comment 17

[Ting-Yu Chou [:ting] (away)]

Just talked to :jerry on IRC:

<ting> Jerry_IRCCloud: Is it possible to interrupt a long reflow whenever all
       the elements in current viewport have been reflowed, so user can see
       updated content even though the full page reflow hasn't been finished?
<Jerry_IRCCloud> ting: I think tiling do the similar thing. It only updates
                 the most important tiles and sends to parent side asap.
<ting> Jerry_IRCCloud: is tiling enabled on desktop?
<Jerry_IRCCloud> ting: I think yes.
<ting> Jerry_IRCCloud: hmm, but I can still see the content is updated after
       full page reflow is done, bug 1257869
<ting> Jerry_IRCCloud: is tiling enabled by a preference?
<Jerry_IRCCloud> ting: yes. but I'm not sure the name
<ting> Jerry_IRCCloud: ok, thanks, mind if I copy those messages to the bug?
<Morris> ting: maybe layers.enable-tiles?
<Jerry_IRCCloud> ting: fine. but I think you should cc |kats|
<ting> Jerry_IRCCloud: great, thanks

Comment 18

[Ting-Yu Chou [:ting] (away)]

layers.enable-tiles is enabled only on macosx:

https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#4410
https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#4434

Comment 19

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #16)
> Is it possible to interrupt a long reflow whenever all the elements in
> current viewport have been reflowed, so user can see updated content even
> though the full page reflow hasn't been finished?

:kats, :jerry told me that tiling does similar thing but I found "layers.enable-tiles" is enabled only on macosx. Are there any plans to land the feature to the other OS? If not, do you have any suggestions? Thanks.

BTW, I don't have a mac to test whether this issue can still be reproduced. :(

Comment 20

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #19)
> BTW, I don't have a mac to test whether this issue can still be reproduced.

I just tried on :wcpan's mac, and it seemed I can't reproduce. So the tiling :jerry mentioned should be helpful.

Comment 21

[Kartikaya Gupta (email:[e-mail])]

(In reply to Ting-Yu Chou [:ting] from comment #19)
> :kats, :jerry told me that tiling does similar thing but I found
> "layers.enable-tiles" is enabled only on macosx. Are there any plans to land
> the feature to the other OS? If not, do you have any suggestions? Thanks.

Correct, tiling is only enabled on OS X right now. No real plans to enable it on other desktop platforms as far as I'm aware, since it ends up being worse for performance overall.

Comment 22

[Kartikaya Gupta (email:[e-mail])]

(In reply to Chris H-C :chutten from comment #0)
> Observation: While resizing, the viewport jitters about as though it's
> resizing to within a rounding error of where it is and then is being snapped
> back. Maybe the destination rect isn't being updated during the resize
> operation?

This sounds like bug 1251778, FWIW. It's still happening and I'm not sure if there is still a bug open for it.

Comment 23

[Ting-Yu Chou [:ting] (away)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #21)
> Correct, tiling is only enabled on OS X right now. No real plans to enable
> it on other desktop platforms as far as I'm aware, since it ends up being
> worse for performance overall.

If so, would you point me in the direction of the code to achieve comment 16? Also what do you think about the idea?

Comment 24

[Kartikaya Gupta (email:[e-mail])]

I'm not sure there is a way to do that. Somebody on the layout team might know better; redirecting needinfo.

Comment 25

[Ting-Yu Chou [:ting] (away)]

Note now the issue can not be reproduced when there're >2 tabs, because the reflow from the last UpdateDimensions IPC for the focused tab will be interrupted by the same IPC for the other TabChild.

Comment 26

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Chou [:ting] from comment #16)
> Is it possible to interrupt a long reflow whenever all the elements in
> current viewport have been reflowed, so user can see updated content even
> though the full page reflow hasn't been finished?

Not really. The problem is, it's impossible to know what "all the elements in the current viewport" are, until you've reflowed them. :)

(Positions are generally kind of sequential, but that's not at all a guarantee.  Some element at the end of the document (a child of something offscreen) could easily have relative positioning with a negative "top" value, or absolute positioning, or a negative margin-top value.  And you can't discover whether it's in the viewport or not until you've reflowed it.)

The "contain" CSS property will eventually provide a way for authors to hint that this sort of leakage isn't possible -- i.e. that offscreen elements must have all of their children also offscreen. We only benefit from that if authors use it, though (and once we have it finished & landed).

In the meantime, Interruptible Reflow does almost what you want, but I believe it's time-based, not "in-the-viewport" based -- and only certain reflow operations are interruptible.

Comment 27

[Ting-Yu Chou [:ting] (away)]

(In reply to Daniel Holbert [:dholbert] from comment #26)
> Not really. The problem is, it's impossible to know what "all the elements
> in the current viewport" are, until you've reflowed them. :)

Wouldn't it be possible for the case of resizing chrome window?

> In the meantime, Interruptible Reflow does almost what you want, but I
> believe it's time-based, not "in-the-viewport" based -- and only certain
> reflow operations are interruptible.

The problem is the last reflow from resizing is not interrupted, so it could take a few seconds to see the updated content. :(

Comment 28

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Chou [:ting] from comment #27)
> Wouldn't it be possible for the case of resizing chrome window?

(I'm interpreting "it" here as "...to only reflow things currently in the viewport, and then repaint, before doing reflow on the rest of the document")

Not really. As an easy pathological case, the document could have a media-query that's conditioned on the viewport size, which completely changes the sizing & positions of everything if the viewport size changes (putting completely different content inside the viewport).  Even without media queries, viewport-size-changes can have drastic effects on the layout of offscreen things, potentially making them appear onscreen.

> The problem is the last reflow from resizing is not interrupted, so it could
> take a few seconds to see the updated content. :(

I'm not particularly familiar with interruptible reflow's heuristics & interception points, but it may be the case that we can & should apply it more aggressively.

Comment 29

[Ting-Yu Chou [:ting] (away)]

I am kind of stuck.

The only message that child receives from parent while resizing is UpdateDimensions, therefore the reflow from the last message won't be interrupted. This is why :chutten saw the scrollbar lag in comment 12.

I was wondering is it possible to interrupt the reflow once the elements in current viewport are reflowed, but turns out it's impossible (comment 26, 28). So the only option I have now is:

Mimic an IPC message for PuppetWidget::HasPendingInputEvent() for every UpdateDimensions message. Because we don't know which is the last UpdateDimensions message, so we need to make one for each of them.

But I don't like this very much as it's a bit hacky. Anyone has better idea?

Comment 30

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Chou [:ting] from comment #29)
> I was wondering is it possible to interrupt the reflow once the elements in
> current viewport are reflowed, but turns out it's impossible (comment 26,
> 28).

I may have been too pessimistic; this might be doable after all, once we have the frame visibility API from bug 1261554.  (When we're doing reflow after a window resize, once a certain threshold has passed, we could skip reflow of any children that weren't visible in our most recent reflow.  This would still incur some jank -- e.g. things that should be coming into view might take a bit longer to become visible (since they wouldn't necessarily be reflowed in the first pass).  But it might be a better experience overall.

Comment 31

[Ting-Yu Chou [:ting] (away)]

Thanks for the information. I'll come back after bug 1261554 gets landed.

Comment 32

[Ting-Yu Chou [:ting] (away)]

Accidentally removed dependency on bug 1255968, sorry for the spam.

Comment 33

[Markus Stange [:mstange]]

(In reply to Ting-Yu Chou [:ting] from comment #29)
> I am kind of stuck.
> 
> The only message that child receives from parent while resizing is
> UpdateDimensions, therefore the reflow from the last message won't be
> interrupted. This is why :chutten saw the scrollbar lag in comment 12.

Do we only ever interrupt reflow if there are pending input events? Shouldn't we interrupt reflow after a certain time even if there aren't any pending input events? I think having some kind of timeout for long reflows would be valuable.

Comment 34

[Ting-Yu Chou [:ting] (away)]

(In reply to Markus Stange [:mstange] from comment #33)
> Do we only ever interrupt reflow if there are pending input events?

From:

  https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2676, and
  https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2577

CheckForInterrupt() checks HavePendingInputEvent() if it has been more than |sInterruptTimeout| ms since we started the reflow, and HavePendingInputEvent() has three work modes: random, counter, and event (default). So I think yes, reflow is interrupted only if there are pending input events.

> Shouldn't we interrupt reflow after a certain time even if there aren't any
> pending input events? I think having some kind of timeout for long reflows
> would be valuable.

Sounds fair, anyone else I should ask to validate it?

Comment 35

[Ting-Yu Chou [:ting] (away)]

What do you think about comment 33 and 34? Thanks.

Comment 36

[Boris Zbarsky [:bzbarsky]]

Reflow interrupts are a tradeoff between throughput and responsiveness.  Worse yet, there are situations in which interrupting every time means we will never complete, since after every interrupt we have to redo some work to get back to where we interrupted, and it's entirely possible for this work to take more than sInterruptTimeout on a large page.

So we decided to make the tradeoff the way we did: we interrupt for responsiveness, but only when there is something to respond to.

Comment 37

[Boris Zbarsky [:bzbarsky]]

Oh, and I guess we _could_ add an unconditional timeout for really long reflows, akin to our slow script timeout, but it would presumably have the same "we no longer offer any guarantee of correctness" semantics and therefore also apply to non-interruptible reflows.  That might be valuable, but doesn't seem like it would help with the problem in this bug.

Comment 38

[Markus Stange [:mstange]]

Makes sense. Then maybe we need a FinishUpdateBounds message that counts as an input event, just so that we end up interrupting the last UpdateBounds reflow?

Comment 39

[Ting-Yu Chou [:ting] (away)]

(In reply to Markus Stange [:mstange] from comment #38)
> Makes sense. Then maybe we need a FinishUpdateBounds message that counts as
> an input event, just so that we end up interrupting the last UpdateBounds
> reflow?

Unfortunately we don't know which is the last UpdateBounds message for sending FinishUpdateBounds. I didn't see mouse events in parent process for pressing, dragging and releasing the border.

Comment 40

[Markus Stange [:mstange]]

On OS X, there are what the documentation calls "Live resize notifications", which would let us send a "Resizing completed" message into Gecko. I wouldn't be surprised if there are similar mechanisms on other platforms.
However, I'm a little confused about the problem we're fixing. Is the scenario: 1. Resize a lot, 2. Stop resizing, 3. Do nothing and wait for the scrollbar to show up in the right place? Does the last reflow get interrupted if, instead of doing nothing, the user starts moving their mouse over web content? If not, why not?

Comment 41

[Ting-Yu Chou [:ting] (away)]

(In reply to Markus Stange [:mstange] from comment #40)
> On OS X, there are what the documentation calls "Live resize notifications",
> which would let us send a "Resizing completed" message into Gecko. I
> wouldn't be surprised if there are similar mechanisms on other platforms.

Thanks for the information, I'll check this.

> However, I'm a little confused about the problem we're fixing. Is the
> scenario: 1. Resize a lot, 2. Stop resizing, 3. Do nothing and wait for the
> scrollbar to show up in the right place?

Yes.

> Does the last reflow get
> interrupted if, instead of doing nothing, the user starts moving their mouse
> over web content? If not, why not?

Yes.

Comment 42

[Seth Fowler [:seth] [:s2h]]

(In reply to Boris Zbarsky [:bz] from comment #36)
> Reflow interrupts are a tradeoff between throughput and responsiveness. 
> Worse yet, there are situations in which interrupting every time means we
> will never complete, since after every interrupt we have to redo some work
> to get back to where we interrupted, and it's entirely possible for this
> work to take more than sInterruptTimeout on a large page.

Couldn't we add the requirement that some progress is actually made each time?

Comment 43

[Seth Fowler [:seth] [:s2h]]

(In reply to Daniel Holbert [:dholbert] from comment #30)
> (In reply to Ting-Yu Chou [:ting] from comment #29)
> > I was wondering is it possible to interrupt the reflow once the elements in
> > current viewport are reflowed, but turns out it's impossible (comment 26,
> > 28).
> 
> I may have been too pessimistic; this might be doable after all, once we
> have the frame visibility API from bug 1261554.  (When we're doing reflow
> after a window resize, once a certain threshold has passed, we could skip
> reflow of any children that weren't visible in our most recent reflow.  This
> would still incur some jank -- e.g. things that should be coming into view
> might take a bit longer to become visible (since they wouldn't necessarily
> be reflowed in the first pass).  But it might be a better experience overall.

Just to be clear, bug 1261554 lets us track which frames are in the displayport, rather than the viewport. However, in an APZ world I think that's the right choice anyway.

Comment 44

[Boris Zbarsky [:bzbarsky]]

> Couldn't we add the requirement that some progress is actually made each time?

We could, but measuring that is nontrivial.

Comment 45

[Ting-Yu Chou [:ting] (away)]

Now I prefer to go with comment 16 & 30, which to interrupt the reflow once current displayport has been reflowed. As this could help not only resize case, but also when load a page.

Comment 46

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #25)
> Note now the issue can not be reproduced when there're >2 tabs, because the
> reflow from the last UpdateDimensions IPC for the focused tab will be
> interrupted by the same IPC for the other TabChild.

(In reply to Ting-Yu Chou [:ting] from comment #41)
> > Does the last reflow get
> > interrupted if, instead of doing nothing, the user starts moving their mouse
> > over web content? If not, why not?
> 
> Yes.

According to above, do this still need to be a P1?

Comment 47

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #41)
> (In reply to Markus Stange [:mstange] from comment #40)
> > On OS X, there are what the documentation calls "Live resize notifications",
> > which would let us send a "Resizing completed" message into Gecko. I
> > wouldn't be surprised if there are similar mechanisms on other platforms.
> 
> Thanks for the information, I'll check this.

On Winfdows there are WM_ENTERSIZEMOVE and WM_EXITSIZEMOVE, however I don't see similar things with GTK.

Comment 48

[Jim Mathies [:jimm]]

No I think we can kick this out of e10s tracking now that bug 1255968 is fixed. We're past the point of bug fixing here as this is something the layout team will get to when they have the time to implement it.

Comment 49

[Chris H-C :chutten]

This is much improved in latest Nightly on both Linux and Windows and is well within acceptable performance for this user. I'm happy to call this "fixed"

If there's still an underlying bug someone wants to tackle with this bug, feel free to reopen.