Closed Bug 910283 Opened 8 years ago Closed 8 years ago

Investigate the cost of CustomizableUI.initialize and TabsInTitlebar.init/_update


(Firefox :: Toolbars and Customization, defect)

Not set





(Reporter: mconley, Assigned: mconley)



(Keywords: perf)

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:

Cc'ing potentially interested parties.
Assignee: nobody → mconley
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.
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, 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.
Summary: Investigate the cost of CustomizableUI.initialize → Investigate the cost of CustomizableUI.initialize and TabsInTitlebar.init
Summary: Investigate the cost of CustomizableUI.initialize and TabsInTitlebar.init → Investigate the cost of CustomizableUI.initialize and TabsInTitlebar.init/_update
I pushed a patch to try to measure the cost of CustomizableUI.initialize, TabsInTitlebar.init, and TabsInTitlebar._update.

Here's the log for XP:

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?
Flags: needinfo?(vdjeric)
Do you have a link to the XP PGO run?
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
Flags: needinfo?(vdjeric)
Whoops - forgot to post it. It's building here:
Here's the compare-talos of that PGO push as compared against some PGO builds of m-c:

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? :)
Flags: needinfo?(vdjeric)
Vladan pointed out the fishy nature of the "original" patch numbers to me, so I'm doing a new m-c baseline (PGO) push:

And we'll compare against that when it's done.
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:

So I guess we can't chalk it all up to those two. :/
Dropping needinfo and marking as RESOLVED, since we've done the investigation.
Closed: 8 years ago
Flags: needinfo?(vdjeric)
Resolution: --- → FIXED
Target Milestone: --- → Future
You need to log in before you can comment on or make changes to this bug.