Closed
Bug 819182
Opened 12 years ago
Closed 12 years ago
startup performance is bad with > 1000 photos
Categories
(Firefox OS Graveyard :: Gaia::Gallery, defect, P1)
Tracking
(blocking-basecamp:+)
RESOLVED
FIXED
blocking-basecamp | + |
People
(Reporter: djf, Assigned: djf)
Details
Attachments
(1 file)
For 809782, I've made it possible for that gallery to display thousands of photos. This is exposed (and exacerbated) a performance problem with the way I enumerate the mediadb and create all the thumbnail elemnets.
With 1600 photos, Gallery takes a couple of minutes to enumerate everything and create thumbnails. Because it is busy with that work, scanning doesn't seem to go quickly, so it does not immediately discover new photos from the camera.
I need to tweak the startup code to make this better, but I don't want to prevent my memory saving patch from landing, so I'm spinning this off as a separate bug.
Assignee | ||
Comment 1•12 years ago
|
||
Assigning this to myself and requesting blocking on it.
Although the description says > 1000 photos, I'd expect this performance problem to be a problem for just a couple hundred photos, so I need to get this resolved.
(I think by fixing the thumbnail memory problem I've made startup speed worse.)
I think I'll actually make this bug block 809782
Updated•12 years ago
|
blocking-basecamp: ? → +
Priority: -- → P1
Assignee | ||
Comment 2•12 years ago
|
||
I've got more details about this now.
The memory improvements I've made in 809782 are not making this bug worse. In fact, they slightly improve startup time. So this bug does not block that one.
There is still a startup time issue however. The basic steps to reproduce is this:
1) Start without gallery running
2) launch camera, take a new photo
3) Tap gallery button in camera app
4) Wait to see the new photo in gallery.
As the number of photos in the gallery grows large, step 4 takes longer and longer.
With 200 photos, it takes about 15 seconds after tapping the gallery button before you see your new photo. There is a chance that this is a regression that occurred when I started displaying videos in the gallery. The enumeration logic got more complex when I started having to interleave photos and videos from two different sources in chronological order.
The gallery starts scanning for new photos as soon as it is launched, and I think that the scanning process is relatively efficient. But at the same time it also starts enumerating all of the photos it knows about and creating thumbnails for those.
It appears that the scanning process does not complete until the enumeration is done. I'm not sure what the reason for this is. Maybe database contention or something?
Anyway, this is a mediadb scanning logic bug or a gallery app enumeration logic bug. I should investigate adding a getAll() method to mediadb (using IndexedDB mozGetAll) and see if I can speed up the enumeration that way. Could I make the enumeration super fast if I read all the records and then created the thumbnails?
I also need to investigate what is going on at the db level. Is the scan waiting to write a record to the db but can't do it until the enumeration completes?
Can I break the enumeration up so that it goes a page at a time, giving the scan a chance to complete?
No longer blocks: 809782
Comment 3•12 years ago
|
||
(In reply to David Flanagan [:djf] from comment #0)
> With 1600 photos, Gallery takes a couple of minutes to enumerate everything
> and create thumbnails. Because it is busy with that work, scanning doesn't
> seem to go quickly, so it does not immediately discover new photos from the
> camera.
A few questions, to determine whether this truly blocks and if we can perhaps mitigate the user scenario:
1) Does this occur every time the user opens the Gallery app?
2) Don't we cache the thumbnails? Or is this a problem even when loading cached thumbnails?
3) Does this occur when a user takes a photo every once in a while and loads Gallery every once in a while (thus getting to >1000 photos)? Or only when loading images on the sd card?
(In reply to David Flanagan [:djf] from comment #2)
> 1) Start without gallery running
> 2) launch camera, take a new photo
> 3) Tap gallery button in camera app
> 4) Wait to see the new photo in gallery.
Can we mitigate this specific use case by loading the images last-in-first-out?
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #3)
>
> A few questions, to determine whether this truly blocks and if we can
> perhaps mitigate the user scenario:
>
If you take away the blocking flag, then I won't be able to work on it, even to mitigate :-)
> 1) Does this occur every time the user opens the Gallery app?
Yes, but is only noticeable if there are > 100 photos or so. My 1000 in the summary line makes the gallery unusable. But this bug is an issue even with fewer.
> 2) Don't we cache the thumbnails? Or is this a problem even when loading
> cached thumbnails?
The thumbnails are cached in indexeddb. For some reason retreiving and displaying them is preventing the new file scan from happening.
> 3) Does this occur when a user takes a photo every once in a while and loads
> Gallery every once in a while (thus getting to >1000 photos)? Or only when
> loading images on the sd card?
I expect this bug to affect any users who uses the camera a lot.
> (In reply to David Flanagan [:djf] from comment #2)
> > 1) Start without gallery running
> > 2) launch camera, take a new photo
> > 3) Tap gallery button in camera app
> > 4) Wait to see the new photo in gallery.
>
> Can we mitigate this specific use case by loading the images
> last-in-first-out?
The gallery app enumerates the photos it knows about in most recent to least recent order. But (this is the bug) while it is doing that the background scan for new photos is not progressing. The mitigation you propose is already in place, but isn't really relevant to this bug.
Database bugs can be scary, but I don't think this is going to be a difficult one. Please just leave it blocking and I'll fix it for C3.
Assignee | ||
Comment 5•12 years ago
|
||
Dominic,
This pull request includes the MediaDB.getAll() method you'll want to use for the music app, and also includes options for tuning MediaDB scanning behavior that you might be interested in.
And it fixes two different bugs of mine. More details on github.
Attachment #692075 -
Flags: review?(dkuo)
Comment 6•12 years ago
|
||
Comment on attachment 692075 [details]
link to patch on github
r+, looks great to me.
please also see the github comments.
And David, thanks for updating the MediaDB so soon, I think there should be no problem for me to use the new methods MediaDB.getAll() and MediaDB.enumerateAll().
Attachment #692075 -
Flags: review?(dkuo) → review+
Assignee | ||
Comment 7•12 years ago
|
||
Fixed in github pr #7008
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•