575093 - 25% TXul regression from hiding the menu bar

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Mike Beltzner [:beltzner, not reading bugmail]]

http://mzl.la/966oi0
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=62b328633b3a&tochange=43b5dea56661

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Resolved?

Improvement: Txul decrease 21.31% on Win7 Firefox
    Previous results:
        112.105 from build 20100628153205 of revision d4deb8d23c80 at 2010-06-28 16:35:00 on talos-r3-w7-022 run # 0
    New results:
        88.2105 from build 20100628161533 of revision 1265868dc033 at 2010-06-28 17:23:00 on talos-r3-w7-010 run # 0
    http://mzl.la/bcQ5Mt
    http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d4deb8d23c80&tochange=1265868dc033

Comment 2

[Jim Mathies [:jimm]]

(In reply to comment #1)
> Resolved?
> 
> Improvement: Txul decrease 21.31% on Win7 Firefox
>     Previous results:
>         112.105 from build 20100628153205 of revision d4deb8d23c80 at
> 2010-06-28 16:35:00 on talos-r3-w7-022 run # 0
>     New results:
>         88.2105 from build 20100628161533 of revision 1265868dc033 at
> 2010-06-28 17:23:00 on talos-r3-w7-010 run # 0
>     http://mzl.la/bcQ5Mt
>    
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d4deb8d23c80&tochange=1265868dc033

Good to see!

Comment 3

[Marco Bonardo [:mak]]

strange That should have not changed anything regarding that measure unless isElementVisible is damn slow

Comment 4

[Dão Gottwald [:dao]]

(In reply to comment #1)
> Resolved?
> 
> Improvement: Txul decrease 21.31% on Win7 Firefox
>     Previous results:
>         112.105 from build 20100628153205 of revision d4deb8d23c80 at
> 2010-06-28 16:35:00 on talos-r3-w7-022 run # 0
>     New results:
>         88.2105 from build 20100628161533 of revision 1265868dc033 at
> 2010-06-28 17:23:00 on talos-r3-w7-010 run # 0
>     http://mzl.la/bcQ5Mt
>    
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d4deb8d23c80&tochange=1265868dc033

Is the three to four per cent difference within the noise range?

Comment 5

[Marco Bonardo [:mak]]

(In reply to comment #4)
> Is the three to four per cent difference within the noise range?

looking at the graph on perftastic, it looks like all boxes are back to previous state.

Comment 6

[Dão Gottwald [:dao]]

Sounds like we can close this.
Not sure which bug solved this, but I think it was one of Jim's widget patches...

Comment 7

[Dão Gottwald [:dao]]

(The pushlog range from comment 1 seems wrong.)

Comment 8

[Markus Stange [:mstange]]

The range in comment 1 seems correct to me after comparing with the graph.

(In reply to comment #3)
> unless isElementVisible is damn slow

It triggers a flush, so it might well have impact.

Comment 9

[Marco Bonardo [:mak]]

makes sense, then it's the patch in bug 575218

Comment 10

[Dão Gottwald [:dao]]

I'm not sure I can follow. Which changeset regressed this if bug 575218 solved it?

Comment 11

[Markus Stange [:mstange]]

The range points to http://hg.mozilla.org/mozilla-central/rev/beedc1e81d8f so this might be a fun interaction between those two things.

My completely uninformed guess is:
The menubar-hiding changeset set .hidden in cases where it didn't do so before, and this added a pending reflow request at a time where there wasn't any before. And then http://hg.mozilla.org/mozilla-central/rev/e6055b0ebd2a removed a flush, so the new pending reflow request is now coalesced with other layout changes that happen later and thus doesn't make a difference any more.