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)
Tracking
(b2g-v2.0 affected, b2g-v2.0M affected, b2g-v2.1 affected, b2g-v2.2 fixed)
RESOLVED
FIXED
2.1 S4 (12sep)
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.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8473180 [details] [review]
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)
Updated•10 years ago
|
Whiteboard: [p=1]
Target Milestone: --- → 2.1 S4 (12sep)
Comment 3•10 years ago
|
||
Comment on attachment 8473180 [details] [review]
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)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8473180 [details] [review]
GitHub pull request URL
(In reply to Steve Chung [:steveck] from comment #3)
> Comment on attachment 8473180 [details] [review]
> 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 5•10 years ago
|
||
Comment on attachment 8473180 [details] [review]
GitHub pull request URL
r=me and this improvement is tremendous, nice! ;)
Attachment #8473180 -
Flags: review?(schung) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 7•10 years ago
|
||
Hi Oleg,
This is also reported by Woodduck partner. Could you also provide a patch for 2.0M?
Thanks!
blocking-b2g: --- → 2.0M+
status-b2g-v2.0:
--- → affected
status-b2g-v2.0M:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Flags: needinfo?(azasypkin)
Comment 9•10 years ago
|
||
Uplift into v2.0 get conflicts. Could you rebase a version for v2.0m if you have chance? Thanks!
Keywords: branch-patch-needed
Assignee | ||
Comment 10•10 years ago
|
||
(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.
Assignee | ||
Comment 11•10 years ago
|
||
Filed a bug 1092112, will provide simple v2.1/v2.0 fix there soon.
Flags: needinfo?(azasypkin)
Keywords: branch-patch-needed
Updated•10 years ago
|
blocking-b2g: 2.0M+ → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•