Closed
Bug 1050129
Opened 10 years ago
Closed 10 years ago
[Gallery] Deleting single image file takes 2.3s
Categories
(Firefox OS Graveyard :: Gaia::Gallery, defect)
Firefox OS Graveyard
Gaia::Gallery
Tracking
(b2g-v2.1 verified, b2g-v2.2 verified)
VERIFIED
FIXED
2.1 S6 (10oct)
People
(Reporter: ranjith253, Assigned: ranjith253)
Details
(Whiteboard: [LibGLA,TD884483,WW,B])
Attachments
(3 files)
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36
Steps to reproduce:
Update to master code.
1. Open gallery , go to select mode.
2. Select any image file & delete.
Actual results:
A single file delete takes approximately around 2.3s .
Expected results:
Single file delete performance should be improved.
Assignee | ||
Comment 1•10 years ago
|
||
Following are my initial analysis for delay of single file deletion.
1. Gallery app sets batchSize: 3 & batchHoldTime: 2000 for scanning.
2. When a file is deleted mediadb checks the deleted files length to batchSize.
3. If length is less than batchsize then mediadb waits for any further delete operation.
4. This waiting time is same as batchHoldTime.
5. Due to this we are having delay in deleting single file.
Requesting David for his opinion on this issue.
Flags: needinfo?(dflanagan)
Assignee | ||
Updated•10 years ago
|
Whiteboard: [LibGLA,TD884483,WW,B]
Assignee | ||
Comment 2•10 years ago
|
||
I tried to fix the issue by setting the batchSize to minimum value before deleting So that mediadb can dispatch delete event immediately . Share your
feedback on this approach.
Attachment #8470748 -
Flags: feedback?(pdahiya)
Attachment #8470748 -
Flags: feedback?(dflanagan)
Comment 3•10 years ago
|
||
Comment on attachment 8470748 [details]
work in progress patch
Ranjith,
Thanks for filing this bug and tracking down the cause. I'm giving feedback- because:
1) MediaDB doesn't really have an API for setting batchSize, and I'm uneasy changing it like that.
2) I think we should consider this a bug in mediadb rather than in gallery. The comments in mediadb make it clear that my intent with batchSize and batchHoldTime was that they would only take effect while we were scanning. So I'm surprised to realize now that file changes while not scanning are also held in batches.
So I think we need to fix this in mediadb. Unfortunately, the video and music apps also use batchSize, so we have to be very careful with changes.
I think the right approach is to change the code that uses batchHoldTime, so that it is only used when we are actually scanning. Note, however, that we don't want to remove the hold time entirely: in the gallery when we delete multiple photos at once we would like for delete event to arrive in a single batch so that the thumbnails get deleted in batches instead of individually.
So I'd suggest modifying mediadb.js to change these lines:
details.pendingNotificationTimer =
setTimeout(function() { sendNotifications(media); },
media.batchHoldTime);
to something like this:
details.pendingNotificationTimer =
setTimeout(function() { sendNotifications(media); },
media.scanning ? media.batchHoldTime : 100);
I have not tried this code myself. I'd be interested to know if it solves the single file deletion issue without compromising the speed of the multi-file delete.
Attachment #8470748 -
Flags: feedback?(dflanagan) → feedback-
Flags: needinfo?(dflanagan)
Assignee | ||
Comment 4•10 years ago
|
||
Hi Djf,
I have tested this PR for single & multiple file delete , I cannot find any speed compromisation. Music & video apps are not using any batchHoldTime while scanning , so I expect there wont be any regression issue with this change.
Attachment #8477843 -
Flags: review?(dflanagan)
Assignee | ||
Comment 5•10 years ago
|
||
Hi David,
Do you have some time to review my patch or can you suggest any peer engineers for review.
Thanks.
Flags: needinfo?(dflanagan)
Comment 6•10 years ago
|
||
Comment on attachment 8477843 [details]
PR
Assigning myself to review and land the patch.
Attachment #8477843 -
Flags: review?(dflanagan) → review?(pdahiya)
Flags: needinfo?(dflanagan)
Comment 7•10 years ago
|
||
Comment on attachment 8477843 [details]
PR
Patch looks good and improves performance while deleting single file and saving edited file in gallery app.
Attachment #8477843 -
Flags: review?(pdahiya) → review+
Comment 8•10 years ago
|
||
Patch landed on master
https://github.com/mozilla-b2g/gaia/commit/87dcf83202920537d9d6a65f6cd37bb3309ad8ce
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Attachment #8470748 -
Flags: feedback?(pdahiya)
Comment 9•10 years ago
|
||
Comment on attachment 8477843 [details]
PR
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Existing issue seen in build 1.4, 2.0, 2.1
[User impact] if declined: It takes ~ 2 secs when a single file is deleted or a new file is created using edit flow in gallery app. This slow performance while deleting an image is one of the reason behind issue reported in bug 1088051.
[Testing completed]: on master
[Risk to taking this patch] (and alternatives if risky): One-line fix that has been running without issues on master for a while now, and it shows significant user visible performance improvement when a file is deleted and after saving a new copy of an edited image.
[String changes made]: None
Attachment #8477843 -
Flags: approval-gaia-v2.1?(bbajaj)
Updated•10 years ago
|
Attachment #8477843 -
Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
Comment 10•10 years ago
|
||
Assignee: nobody → ranjith253
status-b2g-v2.1:
--- → fixed
status-b2g-v2.2:
--- → fixed
Target Milestone: --- → 2.1 S6 (10oct)
Comment 11•10 years ago
|
||
This issue has been verified successfully on Flame2.1&2.2
Verify video:"verify_1050129.mp4".
**From attachment you can see we can delect the picture less than 1 second
Flame2.1 build:
Gaia-Rev ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID 20141201001201
Version 34.0
Device-Name flame
FW-Release 4.4.2
FW-Incremental eng.cltbld.20141201.034405
FW-Date Mon Dec 1 03:44:15 EST 2014
Bootloader L1TC00011880
Flame2.2 bulid:
Gaia-Rev 39214fb22c203e8849aaa1c27b773eeb73212921
Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/08be3008650f
Build-ID 20141201040205
Version 37.0a1
Device-Name flame
FW-Release 4.4.2
FW-Incremental eng.cltbld.20141201.074509
FW-Date Mon Dec 1 07:45:20 EST 2014
Comment 12•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•