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)

defect
Not set
normal

Tracking

(b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S6 (10oct)
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

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.
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)
Whiteboard: [LibGLA,TD884483,WW,B]
Attached file work in progress patch
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 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)
Attached file PR
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)
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 on attachment 8477843 [details] PR Assigning myself to review and land the patch.
Attachment #8477843 - Flags: review?(dflanagan) → review?(pdahiya)
Flags: needinfo?(dflanagan)
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+
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attachment #8470748 - Flags: feedback?(pdahiya)
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)
Attachment #8477843 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
Assignee: nobody → ranjith253
Target Milestone: --- → 2.1 S6 (10oct)
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: