583388 - Delay loading the Tab Candy frame

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Ian Gilman [:iangilman]]

Right now at the bottom of browser/base/content/browser.xul, there's an iframe with id="tab-view", whose source is set to "chrome://browser/content/tabview.html". We want instead for this iframe to start out empty, and be given that source at the right moment in the browser window loading sequence. 

That right moment would be sometime after sessionstore is online. We've been observing the "browser-delayed-startup-finished" event for this purpose (see Storage.onReady() in browser/base/content/tabview/storage.js).

I'm thinking the code that loads the frame (i.e. sets its .src) should live in TabView in browser/base/content/browser-tabview.js, but I'm not sure where it should be called from. 

Once this is in place, we can refine it further, but this is the first step. 

This patch should be for Mardak's tabcandy-central branch.

Comment 1

[Ed Lee :Mardak]

Doing some Ts runs, here's some numbers from each platform for the baseline mozilla-central value, ts from ian's improved startup, ts from not having a src attribute on the iframe, and ts from not having the iframe (but deck is there).

os x 10.6.2
- moz-cen 784
- ian opt 812
- noload  792
- noframe 782

winnt 5.1
- moz-cen 341
- ian opt 382
- noload  357
- noframe 346

winnt 6.1
- moz-cen 430
- ian opt 455
- noload  455
- noframe 427

fedora 12
- moz-cen 450
- ian opt 489
- noload  457
- noframe 455

fedora 12x64
- moz-cen 419
- ian opt 450
- noload  430
- noframe 422

Some reason this build died, but it would probably be 610-615..

os x 10.5.8
- moz-cen 610
- ian opt 664
- noload  615
- noframe ???

Comment 2

[Aza Raskin [:aza]]

With the noframe option, that seems to be roughly the same as moz-central. Or at least, not statistically different than! On average it is only adding 1.3ms. Which means that we probably don't need to play around with exploring a version which doesn't have the deck.

Comment 3

[(no longer active)]

http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/2d1bfc993222

I have no idea where these measurements come from, but I'd be very interested to see measurements with my patch included.

Comment 4

[Michael Yoshitaka Erlewine [:mitcho]]

(In reply to comment #3)
> I have no idea where these measurements come from, but I'd be very interested
> to see measurements with my patch included.

Great!

Mardak will have to push to the Maple branch to get talos results. Mardak?

Comment 5

[Ed Lee :Mardak]

Already done and results are already coming in:
http://tests.themasta.com/tinderboxpushlog/?tree=Maple

Comment 6

[(no longer active)]

(In reply to comment #5)
> Already done and results are already coming in:
> http://tests.themasta.com/tinderboxpushlog/?tree=Maple

So, here are the Ts results (coming from revision 7599ee57af94 on maple):

Linux: 484.0
Linux64: 441.74
OS X: 645.74
OS X 64: 824.79
WinXp: 367.16
Win7: 443.63

But are these numbers good for comparison?  I'm not sure what I need to compare these numbers against, since in my experience due to the noise in out performance test suites, the comparison between the individual numbers is usually meaningless.

Comment 7

[Ed Lee :Mardak]

The initial numbers are in comment 1. I've pushed more changes to the maple tree to get them to run again just to double check.

Comment 8

[Aza Raskin [:aza]]

So we seem to be getting around a 6% regression; we are shooting for a 2% to land I believe.

Comment 9

[(no longer active)]

(In reply to comment #8)
> So we seem to be getting around a 6% regression; we are shooting for a 2% to
> land I believe.

The regression may lie elsewhere.  I think we need to inspect the code to see exactly what things we're doing at startup that we're not doing in m-c builds, and try to optimize by inspection.  We can talk about this at today's meeting.

Comment 10

[(no longer active)]

(In reply to comment #7)
> The initial numbers are in comment 1.

I meant, whether they're coming from maple talos runs or somewhere else...  :-)

> I've pushed more changes to the maple
> tree to get them to run again just to double check.

That's great, thanks!

Comment 11

[Ian Gilman [:iangilman]]

Just for paranoia's sake, those Talos runs from the top of this thread aren't from the same version of mozilla-central as we're using in this most recent maple, right? Our baseline could be off. 

At any rate, ehsan's change should be functionally identical to the "no frame" test above, so I have no idea why we wouldn't be getting the same numbers, unless: 

* It's just noise in the tests
* The latest version of mozilla-central we grabbed has a regression
* We've added something to tabcandy-central, outside of the Tab Candy frame, that's causing the regression

Comment 12

[Ian Gilman [:iangilman]]

We're delaying the frame creation, but we could do more. We should move UIManager.init and ._delayInit out of the frame and into TabView.init, and then create the frame only when the user actually asks for the Tab Candy UI. 

I'll take this on (perhaps with some help from ehsan); reopening this bug for that purpose.

Comment 13

[Ian Gilman [:iangilman]]

Ok, I've pushed the first version of this: 

http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/99097a097e4f

I have a little cleanup to do (properly waiting for the frame to init before operating on it), but that shouldn't affect Ts. 

Mardak, can you push this change to maple and see if it's had any effect? Ehsan, I'd love to hear how it looks on your local Talos tests as well.

Comment 14

[Aza Raskin [:aza]]

I checked in with RelEng and found that the Ts numbers reported are an average of 8 runs (they do 10 runs and throw away the first and last). Wish they gave some more statistics rather than just the first moment, but at least it isn't just one run.

Comment 15

[Michael Yoshitaka Erlewine [:mitcho]]

(In reply to comment #14)
> I checked in with RelEng and found that the Ts numbers reported are an average
> of 8 runs (they do 10 runs and throw away the first and last).

This is news to me, as the wiki says "The page is loaded 10 times, and the shortest startup time is used as the final score."

https://wiki.mozilla.org/Performance:Tinderbox_Tests#Ts:_Startup_time

If the wiki is wrong, it would be nice if RelEng could change it. :/

Comment 16

[Ed Lee :Mardak]

With the latest changes from comment 13 and a mozilla-central with bug 552023 backed out as that caused a Ts regression.

Platform: mozilla-central / f193f519573b
Linux: 440 / 447
Linux64: 411 / 418
OS X: 607 / 624
OS X64: 748 / 772
WinXP: 339 / 341
Win7: 429 / 431

The m-c numbers are pretty similar to the ones from comment 1.

Comment 17

[Ian Gilman [:iangilman]]

So percentage-wise that's:

Linux: 1.5%
Linux64: 1.7%
OS X: 2.8%
OS X64: 3.2%
WinXP: .5%
Win7: .5%

What's up with the Mac? I don't think we're doing anything Mac specific at startup.

At any rate, I feel this is good enough to land.

Comment 18

[Aza Raskin [:aza]]

This is excellent news. It appears we have a regression on average around 1.8%! Way to go team Tab Candy (and special thanks to Ehsan). We still need to figure out a plan on how we are going to get down to 0% regression, (I think Mardak has some ideas there).

Comment 19

[Aza Raskin [:aza]]

This is excellent news. It appears we have a regression on average around 1.8%! Way to go team Tab Candy (and special thanks to Ehsan). We still need to figure out a plan on how we are going to get down to 0% regression, (I think Mardak has some ideas there). But this should be good enough to land.

Comment 20

[Shawn Wilsher :sdwilsh]

(In reply to comment #18)
> This is excellent news. It appears we have a regression on average around 1.8%!
> Way to go team Tab Candy (and special thanks to Ehsan). We still need to figure
> out a plan on how we are going to get down to 0% regression, (I think Mardak
> has some ideas there).
Is this being done in a follow-up bug?  If so, can we get it linked to please?

Comment 21

[Ian Gilman [:iangilman]]

(In reply to comment #20)
> (In reply to comment #18)
> > This is excellent news. It appears we have a regression on average around 1.8%!
> > Way to go team Tab Candy (and special thanks to Ehsan). We still need to figure
> > out a plan on how we are going to get down to 0% regression, (I think Mardak
> > has some ideas there).
> Is this being done in a follow-up bug?  If so, can we get it linked to please?

Further testing has shown that we're within the noise level, but we can't be sure until we land on trunk. In case you haven't, I've filed Bug 586569.

Comment 22

[(no longer active)]

(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #18)
> > > This is excellent news. It appears we have a regression on average around 1.8%!
> > > Way to go team Tab Candy (and special thanks to Ehsan). We still need to figure
> > > out a plan on how we are going to get down to 0% regression, (I think Mardak
> > > has some ideas there).
> > Is this being done in a follow-up bug?  If so, can we get it linked to please?
> 
> Further testing has shown that we're within the noise level, but we can't be
> sure until we land on trunk. In case you haven't, I've filed Bug 586569.

FWIW, bug 586569 has been filed in case we need to optimize Ts more, but I have a strong feeling that I will resolved that bug as WORKSFORME once we see what catlee's script thinks.  :-)

Comment 23

[(no longer active)]

Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".