Closed Bug 583388 Opened 14 years ago Closed 14 years ago

Delay loading the Tab Candy frame

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: iangilman, Assigned: iangilman)

References

Details

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.
Assignee: nobody → ehsan
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 ???
Blocks: 574217
Blocks: 583417
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.
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.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
(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?
Already done and results are already coming in: http://tests.themasta.com/tinderboxpushlog/?tree=Maple
(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.
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.
So we seem to be getting around a 6% regression; we are shooting for a 2% to land I believe.
(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.
(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!
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
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.
Assignee: ehsan → ian
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No longer blocks: 574217
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.
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.
(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. :/
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.
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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).
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
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.
(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?
(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.
(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. :-)
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy. Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.