Closed
Bug 583388
Opened 14 years ago
Closed 14 years ago
Delay loading the Tab Candy frame
Categories
(Firefox Graveyard :: Panorama, defect)
Firefox Graveyard
Panorama
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 | ||
Updated•14 years ago
|
Assignee: nobody → ehsan
Comment 1•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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
Comment 4•14 years ago
|
||
(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•14 years ago
|
||
Already done and results are already coming in:
http://tests.themasta.com/tinderboxpushlog/?tree=Maple
Comment 6•14 years ago
|
||
(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•14 years ago
|
||
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•14 years ago
|
||
So we seem to be getting around a 6% regression; we are shooting for a 2% to land I believe.
Comment 9•14 years ago
|
||
(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•14 years ago
|
||
(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!
Assignee | ||
Comment 11•14 years ago
|
||
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
Assignee | ||
Comment 12•14 years ago
|
||
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 → ---
Assignee | ||
Comment 13•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
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.
Updated•14 years ago
|
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 17•14 years ago
|
||
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 → ---
Comment 18•14 years ago
|
||
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 ago → 14 years ago
Resolution: --- → FIXED
Comment 19•14 years ago
|
||
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•14 years ago
|
||
(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?
Assignee | ||
Comment 21•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
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: -- → ---
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•