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

RESOLVED FIXED in 2.1 S4 (12sep)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: azasypkin, Assigned: azasypkin)

Tracking

unspecified
2.1 S4 (12sep)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

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.
Created attachment 8473180 [details] [review]
GitHub pull request URL
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)
Blocks: 1061491
Whiteboard: [p=1]
Target Milestone: --- → 2.1 S4 (12sep)
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)
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 on attachment 8473180 [details] [review]
GitHub pull request URL

r=me and this improvement is tremendous, nice! ;)
Attachment #8473180 - Flags: review?(schung) → review+
Thanks!

Master: https://github.com/mozilla-b2g/gaia/commit/5914595dfea139db6243cd7765a2adc5f9d70b7f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 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+
status-b2g-v2.0: --- → affected
status-b2g-v2.0M: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → fixed
Flags: needinfo?(azasypkin)

Updated

4 years ago
Duplicate of this bug: 1087991
Uplift into v2.0 get conflicts. Could you rebase a version for v2.0m if you have chance? Thanks!
Keywords: branch-patch-needed
(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)
Keywords: branch-patch-needed
blocking-b2g: 2.0M+ → ---
You need to log in before you can comment on or make changes to this bug.