886528 - BrowserToolbar.from and callees take up disproportionate amount of startup time

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Nathan Froyd [:froydnj]]

Attachment: debug trace from eideticker nytimes-load test

When running eideticker's nytimes load test and using Android's tracing tools, I'm seeing a lot of time spent in BrowserToolbar.from and its callees: ~410ms of CPU time.

MenuPopup.setPanelView and GeckoApp.onCreatePanelMenu seem to be the main time-sucking callees, though MenuPopup.setPanelView quickly descends into Android's libraries.

Do we really need to create all the menu views right away?

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Sriram is probably the person who can answer this best.

Comment 2

[Sriram Ramasubramanian [:sriram]]

Is this in nightly? Nightly landed recent changes to optimize the menu a lot.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Sriram Ramasubramanian [:sriram] from comment #2)
> Is this in nightly? Nightly landed recent changes to optimize the menu a lot.

This was from an m-c build as of last Thursday or so.  I will try a newer build.

Comment 4

[Nathan Froyd [:froydnj]]

I am still seeing this in build from trunk m-c.

Comment 5

[Sriram Ramasubramanian [:sriram]]

BrowserToolbar.from() is the one that initializes the "BrowserToolbar from" (aah! the method names!) the XML, when the app launches. We try to find views in the layout (note: we have 23+ views here, and android recommends having no more than 50 views on screen :P ). Finding views by "findViewById()" is expensive. But again, this is a one-time thing.

Also, we've added checks to make sure that the onCreatePanelMenu is called only for HC+. This is because, they use a custom menu. In case of tablets, some of these menu items have to be added to the url-bar (as action-items). So, the menu has to be inflated when the app starts up, and cannot wait until the first invocation of the menu. I can't find a better way to optimize this part. May be on phones we can suppress this, as there are no action-bar items.

Options:
Move all listeners attaches to onAttachedToWindow(). This will postpone the findViewByIds() until the view is attached to the window. For this, we should have null checks with each and every view -- as gecko could send a message to update the title, when the app has launched in the background and the UI isn't up yet.
  -- This can only reduce the startup (the time to show the app) time. This would still take the same time to attach listeners after attaching to window -- hence might not provide any major win for page load time.

Comment 6

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Since we made the menu's listview when needed, now we can move the inflation of the popup right before showing for the first time.

How much win? Startup speed just increased by 250ms! Earlier it took 311ms for creating and inflating the menu. Now its just 48ms from creation.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Attachment: Part 2: Move initialization

Now that we don't do anything in BrowserToolbar, it's better to move them to GeckoApp. (Note: GeckoApp holds all the logic for the custom menu).

This patch puts it inside intialize(). And initialize() is called after users see something on the screen. So, on tablets, the action-bar items come in, after the basic UI is shown. (Note: This is analogous to android's default menu inflation, as they employ a similar approach). So, the <url-bar><menu-button> will be shown first. And then, the <url-bar> will shrink to make place for refresh and bookmark icons.

Performance note: This piece of code takes about 50-80ms. Now by moving this from BrowserToolbar, we've made the app come up a bit faster. But does the page load start faster? May be.. may be not! This is running on UI thread, right before the huge initialization. So, the Gecko can start rendering in the background.

Android's wicked code:
http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/4.0.3_r1/com/android/internal/policy/impl/PhoneWindow.java#2828
They post a runnable to do the same. This runnable executes after all major things are done. (This is what I meant by "similar approach" -- and is the reason behind action-items showing up late). We could post a runnable too. However, I'm a bit scared as some of the menu items are dependent on gecko state, and we want to initialize menu before we get response from Gecko.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 767354 [details] [diff] [review]
Patch

Make sure you test this on phones/tablets

Comment 9

[Sriram Ramasubramanian [:sriram]]

Tested in on a Nexus and a Nexus 10.
https://hg.mozilla.org/integration/mozilla-inbound/rev/333e96cb832e
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1f379973add

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Let this bake a bit on m-c, but I think it's a good candidate to uplift to aurora.

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/333e96cb832e
https://hg.mozilla.org/mozilla-central/rev/a1f379973add