Last Comment Bug 658852 - When a page is loading, the status panel should appear immediately rather than being faded in (affects page load time)
: When a page is loading, the status panel should appear immediately rather tha...
Status: RESOLVED FIXED
: perf, regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: 4.0 Branch
: All All
: -- normal (vote)
: Firefox 6
Assigned To: Dão Gottwald [:dao]
:
Mentors:
Depends on:
Blocks: 655930 541656
  Show dependency treegraph
 
Reported: 2011-05-22 00:21 PDT by Dão Gottwald [:dao]
Modified: 2011-05-27 10:10 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
wontfix


Attachments
patch (916 bytes, patch)
2011-05-22 00:21 PDT, Dão Gottwald [:dao]
gavin.sharp: review+
dveditz: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2011-05-22 00:21:52 PDT
Created attachment 534280 [details] [diff] [review]
patch

See bug 655930 comment 30

The only part of bug 541656 that could reasonably affect page load time is this part, as far as I can see:

>--- a/browser/base/content/browser.css
>+++ b/browser/base/content/browser.css

> statuspanel {
>   -moz-binding: url("chrome://browser/content/tabbrowser.xml#statuspanel");
>   position: fixed;
>   margin-top: -3em;
>   left: 0;
>+  max-width: 50%;
>+  -moz-transition: opacity 100ms ease-out;
> }
> 
> statuspanel:-moz-locale-dir(ltr)[mirror],
> statuspanel:-moz-locale-dir(rtl):not([mirror]) {
>   left: auto;
>   right: 0;
> }
> 
> statuspanel[label=""] {
>-  visibility: collapse;
>+  opacity: 0;
>+  pointer-events: none;
> }

... the opacity transition in particular.

I pushed the attached patch to Try, WinXP numbers went from 358 to 338:

baseline: http://tbpl.mozilla.org/?tree=Try&rev=177ceed2ffc6
with fix: http://tbpl.mozilla.org/?tree=Try&rev=64e98778133f
Comment 1 Dão Gottwald [:dao] 2011-05-22 00:40:08 PDT
> WinXP numbers went from 358 to 338

Rather to 336. The rerun results don't show up on tbpl for whatever reason.
Comment 2 Dão Gottwald [:dao] 2011-05-22 10:53:53 PDT
http://hg.mozilla.org/mozilla-central/rev/f5dbf215f9ea
Comment 3 Shawn Wilsher :sdwilsh 2011-05-23 08:47:20 PDT
You can get the average of the reruns with this script (which I just run in Scratchpad):
https://gist.github.com/971090/f26a65faf4817acead2b453f217568bfc6c1d936

While this is a nice win, it's still not the entire regression that I found.
Comment 4 Daniel Veditz [:dveditz] 2011-05-23 15:28:05 PDT
Comment on attachment 534280 [details] [diff] [review]
patch

Approved for the mozilla-beta repository, a=dveditz for release-drivers
Comment 5 Dão Gottwald [:dao] 2011-05-24 02:08:54 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/321c235ffaf4
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-24 20:49:24 PDT
If you're not doing a transition on opacity anymore, can you make the statuspanel display:none when it's not needed? That would reduce overhead when it's not visible.
Comment 7 Dão Gottwald [:dao] 2011-05-24 23:24:42 PDT
(In reply to comment #6)
> If you're not doing a transition on opacity anymore, can you make the
> statuspanel display:none when it's not needed? That would reduce overhead
> when it's not visible.

The transition is still there for link target URLs.

If the overhead is substantial, we could probably replace that transition with a longer delay, after which the panel would appear immediately.
Comment 8 Dão Gottwald [:dao] 2011-05-27 10:10:37 PDT
(In reply to comment #3)
> While this is a nice win, it's still not the entire regression that I found.

(In reply to comment #6)
> If you're not doing a transition on opacity anymore, can you make the
> statuspanel display:none when it's not needed? That would reduce overhead
> when it's not visible.

I tested display:none instead of opacity:0 on the try server:
http://hg.mozilla.org/try/rev/9f1d684c834e

This seems to be no win for tp4 at least:
http://perf.snarkfest.net/compare-talos/index.html?oldRevs=d856cbfa6c38&newRev=9f1d684c834e&tests=tp4&submit=true

Note You need to log in before you can comment on or make changes to this bug.