859730 - [sms] Do not render all the threads at startup

Status: RESOLVED WONTFIX

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

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: poc

The current architecture of the sms application use to render all the threads. We know that this does not scale per the contacts app. Fetching 2000 contacts takes 5s for indexedDB without any action of the app itself.

The thread list is probably a bit lighter than the full contacts list (since contact object is heavy) but the current code of the application to display them takes more than 800ms to generate the dom for them for 71 threads. This number is artificially split with setTimeout to free a bit the event loop but that means the last thread can arrive very late.

A better approach could be to render only what is needed on the screen and fetch data when they need to be displayed. This is hard to do right now since the threads list is a big object but a simplified version can be started to makes threads list display faster and more robust to large database.

This is a poc for that. (receiving messages is a bit broken in it).

Comment 1

[Julien Wajsberg [:julienw]]

(a bit broken == it doesn't show up until we restart the app ;) )

Comment 2

[Julien Wajsberg [:julienw]]

Rick, we need several things from gecko before working on this properly. So no need to rush ;)

Comment 3

[Julien Wajsberg [:julienw]]

In particular, I'd like to work first on using properly the cursor introduced in Bug 859662 (follow-up bug to come) so that we have a good foundation to do this bug.

Comment 4

[Borja Salguero [:borjasalguero]]

We talked long time ago about this issue with Vicamo. He is changing from 'Array' to 'Cursor' when retrieving the threads, so we will update the code in Gaia based on the Gecko patch (yesterday was marked as 'Fixed', Im gonna rebuild now for testing it).

Comment 5

[Julien Wajsberg [:julienw]]

Borja, as the title says, this bug is about not rendering all the threads at startup. This means we ultimately want to render only the threads that are actually visible, and render the other only when the user scrolls.

Vivien's initial patch does that already pretty well, but since the cursor work was not there yet, this patch currently fetches everything in an array, so that it can organize it in advance.

I asked in Bug 859732 to be able to fetch the threads with a Filter; the idea is to fetch them by chunks day by day as we scroll, and display only one day (so it's one container) in one pass. That's what Vivien's patch is doing too, and it doesn't work bad.

But I'd like that we carry on the cursor work first, to see if the performance is good enough or not, before working on this bug. That's why I also filed Bug 859917 to have a reference workload so that we can actually judge when it's good enough.

Does that make sense ?

Comment 6

[Borja Salguero [:borjasalguero]]

Hi Julien. I've seen your proposal about the filter and so on, and I like that proposal. On the other hand we could try the 'infinite scroll' already applied in 'getMessages' (it's based in a cursor), and the performance it's good now. We could try both concept and choose the one with better performance, Wdyt?

Comment 7

[Julien Wajsberg [:julienw]]

I really think we should do what we do in getMessages first, as you suggest.

_but_ Vivien and I have a feeling that this is not good enough, even in the thread view. That's why I asked in Bug 859917 to have 1000 sms in one thread. The current x-heavy workload does have (IIRC) 2000 sms in totality, so that means that each thread has actually not so many SMS. But we'll see that for sure after Bug 859917 is done, and only then we'll be able to discuss a strategy.

FYI some good work is being done currently in the Contacts app too, and some have already be done in the Gallery app, and we could leverage what's been done there.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Not tracking since this is way out of scope for v1.1, if product disagrees then bring it back up for discussion but right now tracking this would give it priority over/with other work that is more critical to shipping.

Comment 9

[Julien Wajsberg [:julienw]]

We'll ask for tracking again if/when we have more evidence that this is useful.

Comment 10

[Jon Hylands [:jhylands]]

Julien,

Its been a while - can you comment on whether this is still a valid issue?

Comment 11

[Julien Wajsberg [:julienw]]

Yep this is still valid.

We're still not lazy at all in our initial rendering.

Comment 12

[Julien Wajsberg [:julienw]]

(and unfortunately this _is_ difficult, in case someone that doesn't know the inner workings of SMS want to give it a try)

Comment 13

[Mike Lee [:mlee]]

(In reply to Julien Wajsberg [:julienw] from comment #11)
> ...
> We're still not lazy at all in our initial rendering.

How about now? Still an issue? Is there any plan to actually fix this?

Comment 14

[Julien Wajsberg [:julienw]]

Still not lazy and there is sort of a plan :)

One plan is to first have the database from bug 898364.
Another plan is to stop rendering at one point.

I guess the most important thing is to do a prototype of a list with headers that scrolls efficiently while adding/removing elements.

Comment 15

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 16

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(