994861 - 4.89% tscroll regression on win8 as a result of bug 987680

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

We havea 4.89% tscroll regression on win8 as a result of the landing in comment 27.  

Here is a graph for it:
http://graphs.mozilla.org/graph.html#tests=[[287,131,31]]&sel=1396964336000,1397137136000&displayrange=7&datatype=running

for background on the test:
https://wiki.mozilla.org/Buildbot/Talos/Tests#tscroll.2C_tscrollx

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Bug 896443 added position: relative to root scrollbars on mac and Windows. It should have only been for overlay scrollbars. Since the scrollframe code uses positioned to indicate the scrollbars should be placed over the content (like overlay scrollbars) this made the classic scrollbars be placed higher up in the page.

This fixes the regression for me, and in general doing anything that put the scrollbars back into the background layer fixes the regression.

Comment 2

[Timothy Nikkel (:tnikkel)]

Markus' comment from bug 987680 for reference.

(In reply to Markus Stange [:mstange] from comment #31)
> (In reply to Joel Maher (:jmaher) from comment #30)
> > We havea 4.89% tscroll regression on win8 as a result of the landing in
> > comment 27.  
> 
> We had the inverse improvement on March 20th from
> http://hg.mozilla.org/integration/mozilla-inbound/rev/d5acc6457aaa which
> introduced the bug that this patch fixes:
> http://graphs.mozilla.org/graph.html#tests=[[287,131,31]]&sel=1395273391151.
> 3914,1395298910292.08,6.9595184853549386,8.
> 032138857196667&displayrange=30&datatype=running

Comment 3

[Timothy Nikkel (:tnikkel)]

This now makes sense since http://hg.mozilla.org/integration/mozilla-inbound/rev/d5acc6457aaa accidentally moved scrollbars into the background layer (even overlay scrollbars sometimes), inadvertantly fixing the regression caused by bug 896443.

Comment 4

[Timothy Nikkel (:tnikkel)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e5f7f4427df

Comment 5

[Timothy Nikkel (:tnikkel)]

The automated mailer reported this as a 8.19% improvement, so we fixed the regression and then improved even more.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

great!  Thanks for fixing this.

Comment 7

[Timothy Nikkel (:tnikkel)]

I'll let it get resolved when the sheriffs merge it to central as usual.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8e5f7f4427df

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

rather than open another bug, this seemed to have caused a regression on linux (32 and 64) in tscroll.

linux: 3.6%
linux64: 6.2%

I have done some retriggers to weed out noise and ensure it was this push:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=cbe169081f1c&tochange=376496720a0e&jobname=Ubuntu%20HW%2012.04%20mozilla-inbound%20talos%20svgr

here is a link to the graph:
http://graphs.mozilla.org/graph.html#tests=[[287,131,33]]&sel=1397417505000,1397590305000&displayrange=7&datatype=running

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

:tn, can you common on the tscroll linux regressions?  Is that expected from this?

Comment 11

[Timothy Nikkel (:tnikkel)]

No, very much not expected. The patch only changed the two files
toolkit/themes/osx/global/nativescrollbars.css
and
toolkit/themes/windows/global/xulscrollbars.css
which I'm hoping have no effect on linux builds. I've done several try pushes to see what's up and I'll proceed based on the results of those.

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

thank you, let me know if I can help at all

Comment 13

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

I think windows files are often used on linux as well.

Comment 14

[Timothy Nikkel (:tnikkel)]

Based on my try pushes that appears to be the case thanks.

So I'm guessing this changed the layer structure of the page being tested somehow resulting in slightly worse performance. Not sure if we can do much about that as classic scrollbars are supposed to be in the background layer.

Comment 15

[Timothy Nikkel (:tnikkel)]

There's nothing unusual going on with the talos pages and the scrollbars, and it doesn't matter if the scrollbars get their own layer or not. The only thing that matters if the relative ordering of the scrollbars and the canvas background items in the paint stacking order. scrollbars below canvas bg items -> slow. scrollbars above canvas bg items -> faster.

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for the details here :tn.

Here in datazilla we can view the specifics of each test:
https://datazilla.mozilla.org/?start=1397376632&stop=1397741678&product=Firefox&repository=Mozilla-Inbound-Non-PGO&os=linux&os_version=Ubuntu%2012.04&test=tscrollx&graph_search=8e5f7f4427df&x86_64=false&project=talos

we actually improved iframe.svg, but regressed:
tiled-downscale.html (small)
tiled-fixed-downscale.html (larger)
tiled-fixed.html (larger)
tiled.html (small)

here is the source for tiled-fixed.html:
http://hg.mozilla.org/build/talos/file/35127ce89efd/talos/page_load_test/scroll/tiled-fixed.html

and for tiled-fixed-downscale.html:
http://hg.mozilla.org/build/talos/file/35127ce89efd/talos/page_load_test/scroll/tiled-fixed-downscale.html

of course all the files are here:
http://hg.mozilla.org/build/talos/file/35127ce89efd/talos/page_load_test/scroll

If the *fixed* files have scrollbars below canvas bg items, that would make all make sense.  I am not sure there is anything to do here.

Comment 17

[Timothy Nikkel (:tnikkel)]

(In reply to Joel Maher (:jmaher) from comment #16)
> If the *fixed* files have scrollbars below canvas bg items, that would make
> all make sense.  I am not sure there is anything to do here.

I'm not sure why that would make sense. The canvas background items and the scrollbars don't overlap at all, so one does not actually paint directly on top of the other, it's just the internal ordering of the items. It looks like we are doing the same paint calls of the same areas no matter which order they are painted in.

Comment 18

[Timothy Nikkel (:tnikkel)]

While looking into this I noticed bug 1000266, which coincidentally gets a 5% tscroll win on linux. It's unrelated though, so it doesn't fix the regression in any direct manner (the subtests with fixed bgs gain a lot, other subtests end up slightly behind where we were before, the rest are about neutral).

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

on Aurora after the uplift we have a 3.69% regression on linux32:
http://graphs.mozilla.org/graph.html#tests=[[287,52,33]]&sel=1396793141680,1399385141680&displayrange=30&datatype=running

It seems to be related to this bug as best as I can find by looking at improvements and regressions.  The win we got in bug 1000266 was great on non PGO, but minimized on PGO.

Comment 20

[Timothy Nikkel (:tnikkel)]

That's odd. But this entire bug seems a bit odd. Anyways, bug 1000266 wasn't meant to address this specific regression, it was just something I noticed while looking into it that got us a coincidentally similar improvement.

Looking at 32 and 64 bit data on linux, both PGO and non-PGO series I see some other weird trends. Bug 1000266 wins back something in all cases, but it varies from all of the regression, to maybe a quarter of it. And there is no clear pattern between 64/32bit or pgo/non-pgo.

Non-PGO 32bit, bug 1000266 gets us back to where we were before this regression.
PGO 32bit, bug 1000266 wins back about half the regression.
Non-PGO 64bit, bug 1000266 gets back maybe a quarter of the regression.
PGO, 64bit, bug 1000266 gets back about 75% of the regression.

I also saw another improvement from another bug, bug 1000875 is another win on this test.

64bit, PGO, better than before.
64bit, non-PGO, improved, but still slightly worse then before.
32bit, PGO, back to where we were before.
32bit, non-PGO, better than before.

Still no pattern for 32/64bit or pgo/non-pgo.

Comment 21

[Joel Maher ( :jmaher ) (UTC -8)]

I have been a bit baffled by the inconsistencies here as well.  Usually there is more of a pattern.  Shall we retitle this to be 'the twilight zone' :)

Is there anything we can do (or should do) for linux32 <5% tscroll regression on pgo only?  Should we file a new bug to track that as it is specific and not really related to this bug?