1127078 - Improve Gallery app startup time, particularly on quad-core devices

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[David Flanagan [:djf]]

This bug is the Gaia portion of the larger effort to make Gallery start up faster on quad core devices (bug 1086963).

Comment 1

[David Flanagan [:djf]]

Now that IndexedDB works in workers, my initial plan for this bug was to see whether I could speed up the thumbnail enumeration time by modifying MediaDB to use a worker for indexeddb access.

But after adding some more performance.mark() calls to the startup code, I see that enumeration of the first page of thumbnails typically takes 50-70ms.  But the time between creating the MediaDB object and getting the "ready" event from MediaDB is typically 200ms. So it seems like working to get the MediaDB object ready sooner would be the better approach to take.

Right now we don't create the MediaDB object until after localization (which happens after DOMLoaded) and I don't think we do anything else while the MediaDB is initializing itself, so those 200ms before it is ready are wasted.

Way back in the 1.0 timeframe, I experimented with starting MediaDB sooner. At that point, it did not help at all. The phone was too busy with other stuff while launching, and calling the MediaDB() constructor earlier in the startup sequence did not actually help with overall launch time.

But that was not on a quadcore device. So what I'm going to try first here is to refactor the db initialization code so that the mediadb can be started earlier. My hope is that this will improve overall startup time on the nexus5 without regressing the startup time on the Flame.

In my testing, with about 100 images on the phone, the first screen is typically painted (the "visuallyLoaded" mark) after 650 to 725ms, though the number is quite variable and is sometimes as low as 620 or as high as 800.

Comment 2

[David Flanagan [:djf]]

Here's another possible optimization.  MediaDB currently starts an enumeration of its db in chronological order during initialization because it needs to know what file is newest.  It then stops that enumeration after the first file.  Then gallery does the same enumeration again to find all the thumbnails.

We could modify MediaDB to enumerate a larger batch of db entries and cache them so that they could be quickly returned when the app asks for them.  That only helps with the 50ms enumeration time, however, so is less likely to make a big difference.

Comment 3

[David Flanagan [:djf]]

Needinfo Jim to keep him in the loop because he may need to make similar changes in the Music app.

Comment 4

[David Flanagan [:djf]]

Initial results are disappointing. I moved the call to the MediaDB() constructor to a non-deferred script so that it starts initializing before the document or other scripts load. The MediaDB object is created about 200ms earlier than it would otherwise have been.  But this has no measurable effect on startup time. The 'ready' event from the MediaDB does not arrive until about the same time as it does without the change.

So the extra cores don't seem to help with this startup bottleneck. Maybe using a worker would help in this case.  But unless device storage has been ported to work in workers, it will probably be hard to move just the indexeddb operations to a worker without moving the device storage code to a worker.

Comment 5

[David Flanagan [:djf]]

I've now tried modifying MediaDB so that it sends a "ready" event when IndexedDB is ready but before it has verified that DeviceStorage is available and ready to go.  On the first run of gallery after pushing a new version to the phone this seems to save 200ms!  But on each subsequent run, it improves startup by only 10 to 15ms.  

For that amount of savings I'm not sure it is worth the risk of regressions to change the startup logic...

Comment 6

[David Flanagan [:djf]]

The parent bug of this one references bug 1126119 which is about speeding up hasPendingMessages(). If that call is a bottleneck, then it should be simple to remove it and replace it instead with a check of location.href to determine if the app was launched for a pick activity.  So that is one concrete change I can make here.

Comment 7

[David Flanagan [:djf]]

In order to make progress here, I'm going to need advice from people who know more about gecko internals than I do, so it is needinfo time.

Ben: I think that the IndexedDB API is now available in workers, right? Would you expect that I would see any speedup if I moved my db initialization and enumeration javascript code into a worker?  Or does the DB code already run in its own thread anyway?

Dave: DeviceStorage hasn't become available to workers yet, has it?  Do you have any more general thoughts on how to speed up launch time for the media apps?

Comment 8

[Dave Hylands [:dhylands]]

Bug 916195 (support workers in devic storage) doesn't appear to have been worked on, so the answer appears to still be no.

Speeding up on multi-core is all about being able to divy up work that can be done in parallel on multiple cores.

Using workers is a generic solution. There may be specific things that could be improved. For example, currently enumerate returns one big result.

If the API were changed such that you could query N entries at a time, and each of those queries were performed on a separate thread would probably be alot less work than doing full worker support.

Comment 9

[David Flanagan [:djf]]

Thanks Dave. We've talked about those device storage enumeration improvements before, and I'd love to see them. But for this bug, I only care about getting the first page of thumbnails on the screen as fast as possible, and that happens before I enumerate the filesystem.

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to David Flanagan [:djf] from comment #7)
> Ben: I think that the IndexedDB API is now available in workers, right?
> Would you expect that I would see any speedup if I moved my db
> initialization and enumeration javascript code into a worker?

I guess it depends on how your code is structured and what you want to do with the database... Things like email that need to do a bunch of processing/indexing/updating-in-the-background can definitely benefit because the UI stays responsive while the worker is churning. I'm not sure that the gallery really has that kind of complexity though? Maybe for generating thumbnails or parsing metadata? 

> Or does the DB code already run in its own thread anyway?

The actual database operations happen on their own thread, yeah. But javascript always runs on the main thread unless it's in a worker.

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Sorry, I responded without reading much of the bug. David, let's find some time to chat about the MediaDB so I can get a feel for what it's trying to do?

Comment 12

[David Flanagan [:djf]]

After talking with Ben, I don't think that trying to modify MediaDB to move IndexedDB access to a Worker will help at all.

I don't have any ideas for increasing parallelism at the Gaia level, so I can't really work on this bug in a quad-core specific way, but there are some things I can do to reduce startup time that should help on all devices.

I've tested the following changes on a Nexus 5 and together they were able to bring startup time down from about 660ms to 530ms:

1) replace the use of mozHasPendingMessages with a simple check of window.location.hash

2) remove the tablet code that uses CSS media queries. This is only used for tablet support and I am unable to maintain it. There may be a way to add tablet support back in so that it does not slow down startup on non-tablet devices, but without a tablet I won't be able to do that.

3) modify MediaDB so that it sends its 'ready' event as soon as the database is ready without first checking whether device storage is available. This allows the app to start enumerating thumbnails more quickly, but does create a situation where the user might be able to tap on a thumbnail when the image can not be read from the sdcard because device storage is unavailable.

4) Restructure the Gallery app so that mediadb and minimal startup code are loaded in non-deferred scripts right at the start, in order to get the db initialized more quickly. This gives a good performance gain on quad core, though I suspect that it may not make much difference on dual core. I still need to test that.

I'll break these changes down into separate bugs because they are mostly independent, and I think the uplift decisions for each may need to be made separately.

Comment 13

[David Flanagan [:djf]]

I've filed bugs 1129563, 1129568, 1129572, and 1129584 for each of the above startup time improvements.

Comment 14

[David Flanagan [:djf]]

Attachment: startup_time_tester.js: a useful shim for testing the impact of code changes on average startup time

I have been using this attached file to test the impact of various code changes on startup time.  It measures startup time as the performance.now() number on the first repaint after the "visuallyLoaded" mark, and uses localStorage to keep track of average startup time. Also, when it gets to the "fullyLoaded" mark, it makes the app automatically exit, so that it is easy to launch the app over and over without having to use the task manager to kill it.

Comment 15

[Dave Hylands [:dhylands]]

(In reply to David Flanagan [:djf] from comment #12)
> 3) modify MediaDB so that it sends its 'ready' event as soon as the database
> is ready without first checking whether device storage is available. This
> allows the app to start enumerating thumbnails more quickly, but does create
> a situation where the user might be able to tap on a thumbnail when the
> image can not be read from the sdcard because device storage is unavailable.

This is something that could happen anyways (so the code would need to deal with it).

For example, if you plugged in the usb cable while gallery was open you could tap on a thumbnail and an instant later the sdcard could become unavailable due to sharing being enabled.

Comment 16

[David Flanagan [:djf]]

Attachment: startup_time_tester.js: a shim for testing the impact of code changes on startup time

Comment 17

[David Flanagan [:djf]]

Attachment: startup_time_tester.js: a utility for testing the impact of code changes on startup time, version 3

Attaching a new version of my startup_time_tester.js utility. The last one computed the median time incorrectly.

Comment 18

[David Flanagan [:djf]]

Bug 1129563 and bug 1129572 give us a ~100ms improvement in startup time on both a 1gb flame and on a nexus5-l

We may be able to get a little more with bug 1129584, but that is such a big change that it may not be suitable for uplift to 2.2. So I'm removing that from the list of blockers and will declare victory with the 100ms improvement and close this bug.