Last Comment Bug 910283 - Investigate the cost of CustomizableUI.initialize and TabsInTitlebar.init/_update
: Investigate the cost of CustomizableUI.initialize and TabsInTitlebar.init/_up...
Status: RESOLVED FIXED
: perf
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: unspecified
: x86_64 All
: -- normal (vote)
: Future
Assigned To: Mike Conley (:mconley) - (Needinfo me!)
:
Mentors:
Depends on:
Blocks: australis-ts
  Show dependency treegraph
 
Reported: 2013-08-28 08:57 PDT by Mike Conley (:mconley) - (Needinfo me!)
Modified: 2013-09-10 05:51 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Mike Conley (:mconley) - (Needinfo me!) 2013-08-28 08:57:06 PDT
CustomizableUI is called once (and only once, ever) when the first window is loaded. It might be possible to shave some time off of that for ts_paint.

I'm measuring the cost of CustomizableUI.initialize here:

https://tbpl.mozilla.org/?tree=Try&rev=24e09d0272bb

Cc'ing potentially interested parties.
Comment 1 :Gijs Kruitbosch 2013-08-28 08:58:44 PDT
It may also be possible to load only a subset of the (by now pretty large) JSM and save some that way. So only make available the minimal subset of stuff that initialize needs to do its job.
Comment 2 Mike Conley (:mconley) - (Needinfo me!) 2013-08-28 11:19:53 PDT
So that was a big waste of time, because while on my Linux machine, RecentWindow successfully returned a browser window from which we could get .performance.now(), Windows didn't seem to want to play that game. *sigh*.

Ok, so I've got a patch that works on Windows now - I've tested it on a Windows box and my Linux box. Repushing.

https://tbpl.mozilla.org/?tree=Try&rev=d0ea622eb56a
Comment 3 Mike Conley (:mconley) - (Needinfo me!) 2013-08-29 19:22:12 PDT
I pushed a patch to try to measure the cost of CustomizableUI.initialize, TabsInTitlebar.init, and TabsInTitlebar._update.

Here's the log for XP:

https://tbpl.mozilla.org/php/getParsedLog.php?id=27197468&tree=Try

It looks like about 6.7ms for TabsInTitlebar._update. That, coupled with the ~2ms for CustomizableUI.initialize, accounts for the 1% ts_paint regression we're still seeing. This is because TabsInTitlebar never used to execute for Windows XP, since by default, we were not drawing tabs in the titlebar. Since we are drawing tabs in the titlebar by default, this appears to be the cost for ts_paint.

Vladan - what's the next step here? A lot of effort has gone into making TabsInTitlebar._update as optimized as possible on our end, but perhaps you want to take a crack at it?
Comment 4 Vladan Djeric (:vladan) 2013-08-30 11:40:53 PDT
Do you have a link to the XP PGO run?
Comment 5 Mike Conley (:mconley) - (Needinfo me!) 2013-08-30 13:10:21 PDT
I've re-pushed for PGO:

https://tbpl.mozilla.org/?tree=Try&rev=53e945d6afd9
Comment 6 Vladan Djeric (:vladan) 2013-09-03 14:00:45 PDT
Hey Mike, when you have it, can you post the link to the UX+XP+PGO run with the patch that skips CustomizeUI and tabs in title bar? Thanks
Comment 7 Mike Conley (:mconley) - (Needinfo me!) 2013-09-03 14:02:35 PDT
Whoops - forgot to post it. It's building here:

https://tbpl.mozilla.org/?tree=Try&rev=9812f3382967
Comment 8 Mike Conley (:mconley) - (Needinfo me!) 2013-09-04 08:29:49 PDT
Here's the compare-talos of that PGO push as compared against some PGO builds of m-c:

http://compare-talos.mattn.ca/?oldRevs=e3785e299ab6&newRev=9812f3382967&server=graphs.mozilla.org&submit=true

According to compare-talos, there's no significant difference.

So what do you say, Vladan? Are we cool to close the book on bug 880611? :)
Comment 9 Mike Conley (:mconley) - (Needinfo me!) 2013-09-04 11:38:57 PDT
Vladan pointed out the fishy nature of the "original" patch numbers to me, so I'm doing a new m-c baseline (PGO) push:

https://tbpl.mozilla.org/?tree=Try&rev=c91acdd15540

And we'll compare against that when it's done.
Comment 10 Mike Conley (:mconley) - (Needinfo me!) 2013-09-05 07:08:02 PDT
Hm. So it looks like Vladan's suspicions were right - comparing my disabling of CustomizableUI and TabsInTitlebar with that original try push, we're still seeing a slight regression:

http://compare-talos.mattn.ca/?oldRevs=c91acdd15540&newRev=9812f3382967&server=graphs.mozilla.org&submit=true

So I guess we can't chalk it all up to those two. :/
Comment 11 Mike Conley (:mconley) - (Needinfo me!) 2013-09-09 07:53:23 PDT
Dropping needinfo and marking as RESOLVED, since we've done the investigation.

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