1472546 - Moving position:absolute elements performance far worse (i.e. terrible) with vertical-rl pages than vertical-lr pages

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[wareya]

Attachment: verticalperf.html

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180625141512

Steps to reproduce:

Attached html file reproduces this problem. This problem also affects Japanese popup dictionary addons (like rikaichamp, and mine, nazeka) when used on pages of vertical Japanese text, rendering them nearly unusable because the popup dictionary performs so badly.

Interestingly, performance is fine if the body element has vertical-rl instead of the html element, but this causes the page to start at the "bottom" (left side) instead of the "top" (right side).

The problem is not the location on the page that the element is positioned.

More elaborate pages make the performance problem worse.

The profiler shows that reflowing is taking up a lot of time. It takes up far less, only a couple milliseconds, on vertical-lr pages. It takes up even more time on organic vertical-rl pages with lots of images. https://i.imgur.com/k5gXLLy.png


Actual results:

Performance of html element writing mode vertical-rl is far worse than html element writing mode vertical-lr or body element writing mode vertical-rl.


Expected results:

Performance of html element writing mode vertical-rl should be similar to html element writing mode vertical-lr or body element writing mode vertical-rl.

Comment 1

[wareya]

I left this line in the repro file on accident:

let unused = document.elementFromPoint(0,0);

it has no effect on performance. I added it, because popup dictionaries do similar things, to see if it made a difference (for the worse) and it didn't.

Comment 2

[Gérard Talbot]

wareya,

1- why !important at line 6? I see no reason for !important.

2- window.setInterval(occasional, 1);
The "occasional" function (which creates a visual impression of floating) is called 1000 times per second. "occasional" is a very counter-intuitive name, I'd say, since it is called 1000 times per second!
Is 1000 times really necessary? I mean, is the performance still bad if such function would be called at every 250msec or 500msec (4 or 2 times per second)? The performance bottleneck is unquestionably caused by the occasional function being called 1000 times per second.

The most obvious workaround would be to style that <div id="floating"> with 'position: fixed'. I strongly believe that there would be no noticeable performance issue if done that way. I can create a test for you to verify this for both vertical writing modes.

3- let unused = document.elementFromPoint(0,0);
This instruction is not needed, not required by the visual floating-impression function. But it is nevertheless executed 1000 times per second!

4-

this causes the page to start at the "bottom" (left side)
instead of the "top" (right side).

This is bug 1102175

5-

The profiler shows that reflowing is taking up a lot of time. It
takes up far less, only a couple milliseconds, on vertical-lr pages.

I am not a performance debugger expert but, when describing a performance bug, you need to provide objective description, quantitative measurements, etc... when describing performance issues. And also, such numbers will also be closely relative and closely dependent to the system resources (CPU, RAM, video card, GPU, motherboard frequency, etc.) of the computer testing such webpage. Your computer system resources could be 10 times less powerful than mine.

So, try, as much as possible, to provide concrete measurements, quantifiable data, objective descriptions.

Example given:

Good: 5msec, 2Ghz, 8Mb, 30 images, 50 paragraphs of 97 characters each, rotate 90 degrees, left side of, etc..

Not so good: slow, weak, more, lots of, outside, less,

A screentshot like k5gXLLy.png (performance profile) could be useful to a performance debugger/developer.

Comment 3

[Gérard Talbot]

"delay
The time, in milliseconds (thousandths of a second), the timer should delay in between executions of the specified function or code.
(...)
If this [delay] parameter is less than 10, a value of 10 is used."
https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setInterval#Parameters

Comment 4

[Gérard Talbot]

<div id="floating"> with 'position: fixed'. I strongly believe that there would be no noticeable performance issue if done that way. I can create a test for you to verify this for both vertical writing modes.

http://www.gtalbot.org/BugzillaSection/Bug1472546-MovingAbsPerform-Japn-VRL.html

http://www.gtalbot.org/BugzillaSection/Bug1472546-MovingAbsPerform-Mong-VLR.html

wareya, let me know how these are on your side. Over here, no noticeable CPU spikes or anything above 10% CPU usage when I scroll horizontally, nothing out of the ordinary for me.

Comment 5

[wareya]

Is 1000 times really necessary?

"(...) If this [delay] parameter is less than 10, a value of 10 is used."

I encountered this problem working on a mouseover dictionary extension where the definitions list has to follow the mouse as closely, smoothly, and quickly as possible, regardless of the user's mouse or monitor. So the more times it updates per second the better.

This context was left out of the original report because it doesn't have anything to do with the issue, it's just how I encountered it.

(By the way, the extension does have a throttle for how frequently it tries to update, which is user-configurable. It's not being wasteful.)

The most obvious workaround would be to style that <div id="floating"> with 'position: fixed'.

The div needs to follow the page content automatically (without extra code) if the page gets scrolled, not the window, and it can't interfere with the page's own layout. It cannot be styled as anything but position: absolute.

wareya, let me know how these are on your side. Over here, no noticeable CPU spikes or anything above 10% CPU usage when I scroll horizontally, nothing out of the ordinary for me.

These test cases don't have anything to do with position:absolute styling.

This instruction is not needed

I addressed this. I was reconstructing a much larger test case involving lots of complex behavior, including this instruction. At first I included everything that I suspected might be impacting performance. I accidentally left this instruction in when reducing the test case to only the parts that represented the issue.

So, try, as much as possible, to provide concrete measurements, quantifiable data, objective descriptions.

The profiler screenshot is a concrete measurement. It only shows a large reflow time with this performance issue. Reflow time is a normal tiny value in other cases.

If I change my test case to create 50000 paragraphs of filler text instead of 2000, then vertical-lr looks the same as before (perfectly smooth), but vertical-rl only updates a couple times per second. 50000 causes the test case to take tens of seconds to load, I'm not going to upload a test case like that.

A relatively small difference in performance here would not be unusual. Right-to-left horizontally-scrolling webpages are unusual and rare, and they break a lot of common layout-related assumptions, so a minor performance problem involving them would most likely not be considered a bug. The bug is the fact that the performance difference is "terrible", not exactly how much worse it performs.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Yeah, I agree this is bad. Here's a profile of the page:

https://perfht.ml/2Z20qtw

We seem to be reflowing the whole thing over and over, while usually I would've expected the whole move to be really fast due to RecomputePosition:

https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/layout/base/RestyleManager.cpp#674

Even without that optimization it should be faster.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I'll try to take a look at this when I have some free time.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Finally got some time to dig, sorry for the lag!

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1472546 - Make the RecomputePosition optimization work on vertical-rl writing-modes. r=jfkthame

It wasn't working because it was testing inline size == NS_UNCONSTRAINEDSIZE
rather than block size, so it was taking always the reflow path.

The attached test is on par with the vertical-lr / horizontal-tb cases with
this patch, but takes way over 10s without it.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e851e8aac47
Make the RecomputePosition optimization work on vertical-rl writing-modes. r=jfkthame

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/2e851e8aac47