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

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

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.

Comment 1

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Comment 2

[Oleg Zasypkin [:azasypkin]]

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!

Comment 3

[Steve Chung [:steveck]]

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!

Comment 4

[Oleg Zasypkin [:azasypkin]]

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.

Comment 5

[Steve Chung [:steveck]]

Comment on attachment 8473180 [details] [review]
GitHub pull request URL

r=me and this improvement is tremendous, nice! ;)

Comment 6

[Oleg Zasypkin [:azasypkin]]

Thanks!

Master: https://github.com/mozilla-b2g/gaia/commit/5914595dfea139db6243cd7765a2adc5f9d70b7f

Comment 7

[Josh Cheng [:josh]]

Hi Oleg,
This is also reported by Woodduck partner. Could you also provide a patch for 2.0M?
Thanks!

Comment 9

[Kai-Zhen Li [:seinlin][:kli]]

Uplift into v2.0 get conflicts. Could you rebase a version for v2.0m if you have chance? Thanks!

Comment 10

[Oleg Zasypkin [:azasypkin]]

(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.

Comment 11

[Oleg Zasypkin [:azasypkin]]

Filed a bug 1092112, will provide simple v2.1/v2.0 fix there soon.