820310 - [B2G SMS] Multiple markMessageRead() calls causes significant performance issues

Status: RESOLVED FIXED

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

Description

[Fernando Jiménez Moreno [:ferjm]]

As explained in https://bugzilla.mozilla.org/show_bug.cgi?id=813978#c48 multiple calls to |markMessageRead()| causes a pretty bad performance issue when entering in an SMS thread message view. This bug is supposed to track the required work, in the platform or in the UI side, to improve this performance.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Blocking a blocker, so bb?

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I suggested we try the following approach in the UI:

function markMessagesRead(ids) {
  var id = ids.pop();
  navigator.mozSMS.markMessageRead(id, true).onsuccess = function() {
    if (ids.length) {
      markMessagesRead(ids);
    }
  };
}

If that shows to be too slow, we can look at changing the API.

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Gaia test

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Logcat with gaia test applied

As you can see in the attached logcat, the gaia modifications to chain the calls to |markMessageRead()| are not helping that much :(. For some reason, the first call to |markMessageRead()| when entering in a message view sometimes takes like forever...
I'll try to debug the platform side to see where are we spending that much time.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Logcat without second call to getMessages

I've been doing some more tests and it seems that the problem wasn't in the platform but on the Gaia side. We were calling |getMessages| without a filter (which means that we were following the painful slow path for getting all the messages) after marking the messages as read. After removing this call to |getMessages| the logs show that the times retrieving and marking the messages as read are pretty decent. I am attaching a small sample of this logs.

I've been talking with Borja and he recommended me to wait until Bug 815260 lands before sending any PR with the required modifications to chain the calls to |markMessageRead|.

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

I'll have a patch for this issue this week.

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7149

Pointer to Github pull-request

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 694903 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7149

I'd like to add the functionality to mark the thread as read (visually) as soon as it is clicked as that would be a better UX. How can I do that Borja?

Comment 9

[Borja Salguero [:borjasalguero]]

Only one issue. Check in Github the following:
https://github.com/mozilla-b2g/gaia/pull/7149#issuecomment-11651095

The rest of the code looks great! Thanks for your support ;). Let me know when you fix it and I will review it again.

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Thanks Borja, I've just addressed your feedback and also added the ability to visually mark the thread as read just right after clicking on it to change to the message view (as I requested in comment 8). This way, even if all the messages are not still marked as read in the db, if the user gets back to the thread view, she will see the thread marked as read, which means a better UX.