Closed Bug 1311398 Opened 8 years ago Closed 8 years ago

Investigate delay when Activity Stream panel is displayed

Categories

(Firefox for Android Graveyard :: General, defect, P1)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
All
Android
Type:
defect

Tracking

(firefox52 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

(Whiteboard: [MobileAS])

Attachments

(1 file)

Bug 1311398 - Load public suffix list from disk and extract activity stream labels asynchronously.
58 bytes, text/x-review-board-request
Grisha
: review+
Details 
When display the activity stream panel after app startup there's a delay of 3-4 seconds until data appears. Up to then the panel is just empty.

This happened after the new layout for topsites and highlights landed (bug 1308525). We didn't modify the database queries, so the delay might be caused by something else.
Whiteboard: [MobileAS]
Measuring the query time on my Nexus 6P I see that the top sites loader is slow:

* Highlights (43 ms)
* Top Sites (2519 m)

This is surprising because, using the old panel, top sites load pretty fast.

In addition to that highlights do not show up until top sites have been loaded. They shouldn't need to wait.
The actual slow part here is the bind of the first highlights item which blocks the UI thread and therefore the top sites cursor loader has to wait.

When binding the highlights item we try to extract a label from the URL. This uses the PublicSuffix util, which on class load, needs to parse the list of public suffixes into a set. This is slow and takes about 2.5 seconds on the Nexus 6P.
The current format of the public suffix list is optimized to be small and we need to parse it in order to obtain the set (2500ms on a Nexus 6P). For testing I stored the full list in assets and loaded it into the set on the first use. This brought the time down to 30-50ms. I want to try some other options:
* Explore using the list we already ship, but it is generated c++ code (Maybe we can access it using JNI)
* Having the List in SQLite - Unlikely that this is faster than the flat file - especially as we can keep the file contents in memory

If we are going to read from disk then the methods extracting the public suffix need to be asynchronous.
Iteration: --- → 1.7
* Building the list at runtime doesn't work: Method too large
* Creating the list from a large string doesn't work either: String too long
Comment on attachment 8803838 [details]
Bug 1311398 - Load public suffix list from disk and extract activity stream labels asynchronously.

https://reviewboard.mozilla.org/r/87978/#review88246

Ugh, crazy stuff. I take it you've tried concatinating multiple strings (just at their length limit)? It's very surprising that reading from disk will be faster than that... Or was that the "method too large" comment you've made?
Attachment #8803838 - Flags: review?(gkruglov) → review+
(In reply to :Grisha Kruglov from comment #6)
> Ugh, crazy stuff. I take it you've tried concatinating multiple strings
> (just at their length limit)? It's very surprising that reading from disk
> will be faster than that... Or was that the "method too large" comment
> you've made?

Yeah, at first I tried to add all domains as single strings to the list. This resulted in the method to be too large. From what I found online it seems like strings can get very large at runtime, but string literals are much more constrained.
Flags: needinfo?(snorp)
(The NI is actually for this comment)

Before landing this I want to see if we can make any use of the native implementation.

@snorp: I want to find the public suffix in a string containing a domain. Our build process generates code from the public suffix list [1] and imports it into nsEffectiveTLDService.cpp to provide some of the functionality. Is it possible to access some of this code (or this compiled public suffix list) from JNI? We need this for new home panel code and therefore can't rely on Gecko running. Right now we have a Java implementation and for that need to ship the public suffix list a second time (in assets) and that's somehow ridiculous.

[1] https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-pc-linux-gnu/netwerk/dns/etld_data.inc
[2] https://dxr.mozilla.org/mozilla-central/source/netwerk/dns/nsEffectiveTLDService.cpp#35
(For the record: I tried accessing the native code but ran into various compile errors and wasn't really able to access nsEffectiveTLDService or the list).
We talked on IRC. Accessing the code without having Gecko running first is not really feasible.
Flags: needinfo?(snorp)
I'll land this patch now and then let's see whether we see an increase after the list has been compressed.
Iteration: 1.7 → 1.8
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2016d6d9e353
Load public suffix list from disk and extract activity stream labels asynchronously. r=Grisha
https://hg.mozilla.org/mozilla-central/rev/2016d6d9e353
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: