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

RESOLVED FIXED in Firefox 25



Firefox for Android
4 years ago
4 years ago


(Reporter: froydnj, Assigned: sriram)


(Blocks: 1 bug)

Firefox 25
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(3 attachments)



4 years ago
Created attachment 766893 [details]
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?
Sriram is probably the person who can answer this best.
Flags: needinfo?(sriram)

Comment 2

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


4 years ago
Blocks: 807322

Comment 3

4 years ago
(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

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

Comment 5

4 years ago
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.

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.
Flags: needinfo?(sriram)

Comment 6

4 years ago
Created attachment 767354 [details] [diff] [review]

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.
Attachment #767354 - Flags: review?(mark.finkle)

Comment 7

4 years ago
Created attachment 767434 [details] [diff] [review]
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:
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.
Attachment #767434 - Flags: review?(mark.finkle)
Comment on attachment 767354 [details] [diff] [review]

Make sure you test this on phones/tablets
Attachment #767354 - Flags: review?(mark.finkle) → review+
Attachment #767434 - Flags: review?(mark.finkle) → review+

Comment 9

4 years ago
Tested in on a Nexus and a Nexus 10.
Let this bake a bit on m-c, but I think it's a good candidate to uplift to aurora.
Assignee: nobody → sriram
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25


4 years ago
Depends on: 888381
You need to log in before you can comment on or make changes to this bug.