1020469 - vh is slow after a resize

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Julien Wajsberg [:julienw]]

Attachment: 358bc85c112ecb230c2f30f41b064aee8a2b8d89.patch

I tried using the "vh" unit in a "max-height" property (inside a "calc") for Gaia's SMS app, and it seemed slow when reflowing (for example, when resizing the application when the keyboard is displayed/hidden).

I profiled on a Buri device, and I get [1]. We see clearly that the resizing operation takes ~450ms (RecvUpdateDimensions). If, instead, I use a % unit instead of vh, then I get the profile [2], and we see the same operation takes 19ms.

[1] https://people.mozilla.org/~bgirard/cleopatra/#report=5558e30c8a27e8578b683b95ff7bb9d39ecc203e
[2] https://people.mozilla.org/~bgirard/cleopatra/#report=f0b8db92cf56b1da5e9bfe562bcded78763a3d93


If you want to try it yourself, it's possible in Firefox too. Here is how:

1. apply attached patch on your Gaia repository
2. generate a good profile using:
  PROFILE_FOLDER=profile-sms DEBUG=1 DESKTOP=0 make
3. launch Firefox nightly with this profile
  /path/to/nightly/firefox --no-remote -profile ./profile-sms
4. navigate to http://sms.gaiamobile.org:8080
5. switch to responsive mode (ctrl + shift + m)
6. open a message thread
7. resize the application's window

=> you'll see this is very slow.
=> You can compare with Gaia master instead, you'll see it's very different. (you just need to change or clean up the branch and reload in firefox to compare)
=> You can change the max-height property using "%" and see it's way faster.

(Note: I can provide newer patches if the master code changes; alternatively, you can fetch commit [3] from my repository).

[3] https://github.com/julienw/gaia/commit/358bc85c112ecb230c2f30f41b064aee8a2b8d89

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is a well-known limitation of viewport-relative units.  They cause all style data to be thrown out and recomputed on viewport resize, because the assumption is that viewport resizes are rare.  So a viewport resize on a page that uses such units has to do a lot more work than on a page that doesn't do that.

I could have sworn we had existing bugs with discussion about this.

(Note that the profiles linked above don't have obvious resize stuff in them that I can see...)

Comment 2

[Julien Wajsberg [:julienw]]

(In reply to Boris Zbarsky [:bz] from comment #1)
> This is a well-known limitation of viewport-relative units.  They cause all
> style data to be thrown out and recomputed on viewport resize, because the
> assumption is that viewport resizes are rare.  So a viewport resize on a
> page that uses such units has to do a lot more work than on a page that
> doesn't do that.

I've seen other cases where using "vh" was not producing such slowness ([1]). Is it especially slow here because of the flexible layout?

[1] https://github.com/mozilla-b2g/gaia/pull/19477/files

> 
> I could have sworn we had existing bugs with discussion about this.
> 
> (Note that the profiles linked above don't have obvious resize stuff in them
> that I can see...)

Look to the "Messages" line, what's important is the latest spike of activity.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> I've seen other cases where using "vh" was not producing such slowness 

Depends on how long a style recomputation takes, if nothing else, right?

> Is it especially slow here because of the flexible layout?

I don't know.  That's why I'm looking at the profiles.

> Look to the "Messages" line, what's important is the latest spike of activity.

That's showing me a bunch of waiting and painting, but no obvious anything under RecvUpdateDimensions, unless I'm just being blind...

Comment 4

[Julien Wajsberg [:julienw]]

(In reply to Boris Zbarsky [:bz] from comment #3)
> > I've seen other cases where using "vh" was not producing such slowness 
> 
> Depends on how long a style recomputation takes, if nothing else, right?

Yes, but at least the DOM is not that different between both cases. (the styles are probably much different though)

> 
> > Is it especially slow here because of the flexible layout?
> 
> I don't know.  That's why I'm looking at the profiles.
> 
> > Look to the "Messages" line, what's important is the latest spike of activity.
> 
> That's showing me a bunch of waiting and painting, but no obvious anything
> under RecvUpdateDimensions, unless I'm just being blind...

In the first profile, you see a lot of things, especially "ComputeStyleChangeFor" which calls "FileRules" and "CalcStyleDifference".

In the second profile, not so much, because it works fine.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Talked about this with Julien, sorted out my profiler confusion.

The case mentioned in comment 2 doesn't have that rule matching anything, so no actual _use_ of viewport-relative units.

As far as making these faster, what we'd really need to do as a minimum is to mark any style context and rulenode affected by them, then on viewport size change mark those rulenodes as "dead" somehow and recompute styles for the nodes that pointed to those style contexts.  So maybe walk over the frame tree, and when we find a frame with the relevant bit set go ahead and reresolve that subtree.  The "dead" marking on the rulenodes would ensure we don't use them for these new style contexts.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It might also be reasonable to suggest that vh and vw be avoided in our own UI.

I supported implementing them because they were a common author request even though they were a bit of a hack and unlikely to perform well; in hindsight I think implementing them was probably a mistake, and we should push back harder on easy-but-slow features in the CSS WG.