Closed Bug 1053952 Opened 10 years ago Closed 10 years ago

[Messages][Refactoring] Delete all messages at once when deleting threads

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.0 affected, b2g-v2.0M affected, b2g-v2.1 affected, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S4 (12sep)
Tracking Status
b2g-v2.0 --- affected
b2g-v2.0M --- affected
b2g-v2.1 --- affected
b2g-v2.2 --- fixed

People

(Reporter: azasypkin, Assigned: azasypkin)

References

Details

(Whiteboard: [p=1])

Attachments

(1 file)

Messaging API "delete" method accepts array of message IDs and we should utilize this ability when user removes threads. Having only one "delete" call should definitely improve performance. Below are some measurements for Flame (batch deletion patch vs master), the target time is the time between user confirmation to delete and moment when waiting screen is closed and control is returned to the user: Workloads | Light workload | Medium workload | High workload | ------------------------------------------------------------------------------------------- | ~696ms vs ~3065ms | ~1358ms vs ~7653ms | ~2622ms vs ~15389ms | Remove 1 thread | 59 messages deleted | 124 messages deleted | 245 messages deleted | | | | | ------------------------------------------------------------------------------------------- | ~1898ms vs ~14801ms | ~5020ms vs ~43147ms | ~9303ms vs ~73212ms | Remove 10 threads | 285 messages deleted | 668 messages deleted | 1261 messages deleted | | | | | ------------------------------------------------------------------------------------------- | ~2731ms vs ~23553ms | ~7653ms vs ~60121ms | ~13700ms vs ~118073ms | Remove all threads | 400 messages deleted | 1000 messages deleted | 2000 messages deleted | | 37 threads deleted | 90 threads deleted | 181 threads deleted | So it's clear that we should make this change.
Attached file GitHub pull request URL (deleted) —
Comment on attachment 8473180 [details] GitHub pull request URL Hey Steve, Here is one more _not urgent_ review for the time when your review queue is empty :) Rebased it on master and it looks ready for review. Thanks!
Attachment #8473180 - Flags: review?(schung)
Blocks: 1061491
Whiteboard: [p=1]
Target Milestone: --- → 2.1 S4 (12sep)
Comment on attachment 8473180 [details] GitHub pull request URL Hi Oleg, some nits in github but it looks so great! Only one issue about the draft, and I think we could handle it in Threads.delete instead of in Threads list deletion, thanks!
Attachment #8473180 - Flags: review?(schung)
Comment on attachment 8473180 [details] GitHub pull request URL (In reply to Steve Chung [:steveck] from comment #3) > Comment on attachment 8473180 [details] > GitHub pull request URL > > Hi Oleg, some nits in github but it looks so great! Only one issue about the > draft, and I think we could handle it in Threads.delete instead of in > Threads list deletion, thanks! Thanks for review! Updated PR with fixes according to your comments.
Attachment #8473180 - Flags: review?(schung)
Comment on attachment 8473180 [details] GitHub pull request URL r=me and this improvement is tremendous, nice! ;)
Attachment #8473180 - Flags: review?(schung) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Hi Oleg, This is also reported by Woodduck partner. Could you also provide a patch for 2.0M? Thanks!
blocking-b2g: --- → 2.0M+
Flags: needinfo?(azasypkin)
Uplift into v2.0 get conflicts. Could you rebase a version for v2.0m if you have chance? Thanks!
(In reply to Kai-Zhen Li [:seinlin] from comment #9) > Uplift into v2.0 get conflicts. Could you rebase a version for v2.0m if you > have chance? Thanks! This patch is a bit risky to uplift, so we'll try to find simpler fix\solution for partner bug 1087991 first. Keeping ni?=me to provide more details.
Filed a bug 1092112, will provide simple v2.1/v2.0 fix there soon.
Flags: needinfo?(azasypkin)
blocking-b2g: 2.0M+ → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: