Closed Bug 697747 Opened 8 years ago Closed 8 years ago

Takes to long to show awesomescreen results, especially on startup

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
firefox13 --- verified

People

(Reporter: wesj, Assigned: lucasr)

Details

Attachments

(4 files, 1 obsolete file)

One of the "fast startup" goals of Native Fennec was that you could very quickly start digging through your awesome screen results. There's a big delay on my phone (Droid X) between the time I tap on the awesome bar and results actually appear for me though.

Some data from timing logs I put in BrowserToolbar.java and AwesomBarTabs.java using

Log.w("TIMING", "<something>" + new Date().getTime());

During Startup:
Class            Time          Method           deltaT  
BrowserToolbar   1319734280681 OnClickListener  0
HistoryQueryTask 1319734281374 doInBackground   693
HistoryQueryTask 1319734281866 onPostExecute    1185

BrowserToolbar   1319734695755 OnClickListener  0
HistoryQueryTask 1319734696631 doInBackground   876
HistoryQueryTask 1319734697442 onPostExecute    1687

Close awesomescreen and open again:
BrowserToolbar   1319734356340 OnClickListener  0
HistoryQueryTask 1319734356442 doInBackground   102
HistoryQueryTask 1319734356903 onPostExecute    563
Here a few improvements I'm working on:
 - Limit the number of rows returned by the frecency and history queries (DONE)
 - Only perform the db query on the frecency tab when awesomebar activity is created. Right now we're running 3 queries at the same on awesomebar startup which might be causing delays. The queries for bookmarks and history would be done only if user actually clicks on their respective tabs. (ALMOST DONE)
 - Move the history adapter splitting code (to put each history entry under Today, Yesterday, Week, Older groups) to run on a background thread. This will ensure we're not doing heavy work on main thread.

Wes, could you send me your patch that adds those timers? This way I can post the numbers I get with my patches.
Assignee: nobody → lucasr.at.mozilla
Attachment #570242 - Flags: review?(mark.finkle)
Those patches makes AwesomeBar shows results perceivably quicker on my Galaxy S. Wes, could you try those on your Droid X?
Fixes a crash I spotted while testing.
Attachment #570242 - Attachment is obsolete: true
Attachment #570242 - Flags: review?(mark.finkle)
Attachment #570244 - Flags: review?(mark.finkle)
Comment on attachment 570240 [details] [diff] [review]
(1/3) Limit the number of rows returned in AwesomeBar queries

So this patch is only limiting All Pages and History, right? We don't want to limit Bookmarks.
Attachment #570240 - Flags: review?(mark.finkle) → review+
Attachment #570241 - Flags: review?(mark.finkle) → review+
Attachment #570244 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #7)
> Comment on attachment 570240 [details] [diff] [review] [diff] [details] [review]
> (1/3) Limit the number of rows returned in AwesomeBar queries
> 
> So this patch is only limiting All Pages and History, right? We don't want
> to limit Bookmarks.

Yes, only all pages and history. We'll have to deal with the case of having a very long list of bookmarks at some point. Not high priority for now.

We might want to implement some sort of "incremental list adapter" in the future to only load a small part of the list initially and load more content as the user scrolls. Also not high priority for now I guess.
Patch 3 (Only query All Pages on startup) should have a positive impact when opening the awesomebar. Patch 2 (Move history group splitting to background thread) should help with responsiveness, but might not help any delaying in "seeing" data displayed in the list. We'll see.

(In reply to Lucas Rocha (:lucasr) from comment #8)

> We might want to implement some sort of "incremental list adapter" in the
> future to only load a small part of the list initially and load more content
> as the user scrolls. Also not high priority for now I guess.

Yes. This would be the optimal solution and hardest to implement :)
(In reply to Mark Finkle (:mfinkle) from comment #9)
> Patch 3 (Only query All Pages on startup) should have a positive impact when
> opening the awesomebar. Patch 2 (Move history group splitting to background
> thread) should help with responsiveness, but might not help any delaying in
> "seeing" data displayed in the list. We'll see.
> 
> (In reply to Lucas Rocha (:lucasr) from comment #8)
> 
> > We might want to implement some sort of "incremental list adapter" in the
> > future to only load a small part of the list initially and load more content
> > as the user scrolls. Also not high priority for now I guess.
> 
> Yes. This would be the optimal solution and hardest to implement :)

Actually, after thinking a bit more about this, given that we're loading a relatively small number of items per query (100 items is fairly small), I'd doubt we ever need to do a full incremental load adapter. What might want to try though is only loading the favicon images for the visible rows in the lists (which wouldn't be too complicated to do). We have to check if favicons are actually impacting performance though.
Awesome lucas. I'll test in a bit when I get to work. I can do some quick things like removing favicons (entirely) from the query/workload if we want those numbers too.
Pushed:
http://hg.mozilla.org/projects/birch/rev/12f6c3a2b665
http://hg.mozilla.org/projects/birch/rev/403d217a6c09
http://hg.mozilla.org/projects/birch/rev/ddc7b89fa4d6

Wes, I'll close this bug for now (this is the kind of bug that could stay open forever :-P). Please post your numbers anyway. We can file separate bugs for specific follow-up improvements.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attached patch Timing patchSplinter Review
Hmm... so yeah. I only glanced at this briefly to guess how it was all running. We don't do the history query anymore, which I think is helping somewhat, but I still see delays in showing the list.

I tried logging times in some different places using the attached patch. Looks like this query is just slow, and getting to the "runQuery" function is also taking a bit on startup.

Not sure what you want to do with this lucas. I'm going to play with removing favicon stuff from the query as well to see if that helps...

Startup:
BrowserToolbar	onClick	1319821732804	0
AwesomeBarTabs	addAllPagesTab	1319821732858	54
FilterQueryProvider	runQueryStart	1319821733434	630
FilterQueryProvider	runQueryEnd	1319821733498	694 // not sure how the query was so fast here

BrowserToolbar	onClick	1319822097089	0
AwesomeBarTabs	addAllPagesTab	1319822097144	55
FilterQueryProvider	runQueryStart	1319822097676	587
FilterQueryProvider	runQueryEnd	1319822098220	1131

BrowserToolbar	onClick	1319822452642	0
AwesomeBarTabs	addAllPagesTab	1319822452699	57
FilterQueryProvider	runQueryStart	1319822453545	903
FilterQueryProvider	runQueryEnd	1319822453944	1302

Open second time:
BrowserToolbar	onClick	1319821642066	0
AwesomeBarTabs	addAllPagesTab	1319821642126	60
FilterQueryProvider	runQueryStart	1319821642145	79
FilterQueryProvider	runQueryEnd	1319821642511	445
			
BrowserToolbar	onClick	1319821917824	0
AwesomeBarTabs	addAllPagesTab	1319821917901	77
FilterQueryProvider	runQueryStart	1319821917930	106
FilterQueryProvider	runQueryEnd	1319821918364	540
			
BrowserToolbar	onClick	1319822009744	0
AwesomeBarTabs	addAllPagesTab	1319822009822	78
FilterQueryProvider	runQueryStart	1319822009840	96
FilterQueryProvider	runQueryEnd	1319822010247	503
Wes, could you try reducing MAX_RESULTS in AwesomeBarTabs to, say, 50 and see if it makes any perceivable difference? It might just be that building the UI side (not the query itself) of ListView takes more time than we'd expect somehow.
I did drop max results from 100 to 10 the other day and saw about an order of magnitude increase in the speed of the query. I also played with commenting out this call:

http://hg.mozilla.org/projects/birch/file/1f61c869b696/embedding/android/GeckoApp.java#l392

to prevent Gecko loading, and I believe that reduced the time to runQueryStart drastically. I'm not that suprised by that, but don't 100% trust those statements and want to test them again.
Verified fixed on:

Firefox 13.0a1 (2012-02-17)
20120217031227
http://hg.mozilla.org/mozilla-central/rev/2271cb92cc05

--
Device: Motorola Droid PRO
OS: Android 2.3.3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.