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

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

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 (he/him)]

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) (:⚙️)]

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) (:⚙️)]

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)]

Do you have a link to the XP PGO run?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

I've re-pushed for PGO:

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

Comment 6

[Vladan Djeric (:vladan)]

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) (:⚙️)]

Whoops - forgot to post it. It's building here:

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

Comment 8

[Mike Conley (:mconley) (:⚙️)]

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) (:⚙️)]

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) (:⚙️)]

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) (:⚙️)]

Dropping needinfo and marking as RESOLVED, since we've done the investigation.