Closed Bug 903763 Opened 7 years ago Closed 6 years ago

[Messages] avoid css recalculations while rendering a thread

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 887198

People

(Reporter: julienw, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [c=progress p= s= u=][p(2.1S5)=1])

when profiling the thread rendering with Gabriele, we clearly see that when there are enough messages in the thread, there is a big overhead due to the CSS styles refresh.

This probably happens because we append a node to the DOM for each message, even if it's hidden. Since we're doing this asynchronously because/thanks to the SMS API cursor, the style refresh is done after each message is added to the DOM.

In this bug I'd like to try rendering the hidden messages in a document fragment instead, and append the document fragment only once it's finished.

This could be quite difficult, because in the rendering functions we're trying to find the correct location to append the new message to, but it could be difficult to do if we have some nodes in the DOM and some nodes in a fragment.
Keywords: perf
Thinking more of this, I think we can merely store the messages information in an array and render everything we have so far when rendering the first chunk. And same for the last chunk.
Summary: [sms][mms] use a DocumentFragment when rendering messages in a thread → [messages] avoid css recalculations while rendering a thread
(In reply to Julien Wajsberg [:julienw] from comment #1)
> Thinking more of this, I think we can merely store the messages information
> in an array and render everything we have so far when rendering the first
> chunk. And same for the last chunk.

Now we're talking :)
Summary: [messages] avoid css recalculations while rendering a thread → [Messages] avoid css recalculations while rendering a thread
Assignee: nobody → gnarf37
Status: NEW → ASSIGNED
Whiteboard: [c= p= s= u=]
Another (untested) lead suggested by Vivien: if we add a node to a parent which is already "display: none", then we won't have the style overhead either.

I still think comment 1 makes more sense in the long term.
Assignee: gnarf37 → nobody
Duplicate of this bug: 948493
So Bug 948493 was set as a dupe of this bug.  I'm not sure that's accurate.  I *just* started dog fooding a Keon and a Peak and I'm seeing a huge lag in the message input box receiving focus and accepting input.

I do NOT have a large backlog of messages.  I was sending my first few text messages with the device.  I'm using v1.2 stable from geeksphone so this may have been fixed already.  It only happens immediately after opening the app or switching to the app.

The scenario that is most annoying me is that I want to send a text message to a friend and I launch the app.  I sit there tapping the message input box for 5 seconds before it receives focus.  I then type out my message and send it then hit the home button to check something else.  When my friend replies, I switch to the message app to reply and again, I sit there tapping the message input box for 5 seconds before it receives focus and starts accepting input.

The end result is that having a back-and-forth text message conversation with somebody is extremely annoying.
(In reply to Dave Huseby [:huseby] from comment #5)
> The scenario that is most annoying me is that I want to send a text message
> to a friend and I launch the app.  I sit there tapping the message input box
> for 5 seconds before it receives focus.

This sounds a lot worse than what we usually run into. We've seen 5 second lags but only with 1000+ messages in a thread. Can you check which gaia version is used in that build?
I'm reopening the other one for now then!
Julien, we know a lot of changes have and are happening in Messages. Please retest and confirm if this is still an issue.

Thanks,
FxOS Perf Triage
Flags: needinfo?(felash)
Priority: -- → P3
Whiteboard: [c= p= s= u=] → [c=progress p= s= u=]
Hi Mike, yes, we're still having an issue here, nothing sadly changed yet.
Flags: needinfo?(felash)
See Also: → 887198
Adding it in this sprint's workload but we know it will take more time, so I set target milestone for the next sprint.
Whiteboard: [c=progress p= s= u=] → [c=progress p= s= u=][p=1]
Target Milestone: --- → 2.1 S6 (10oct)
Target Milestone: 2.1 S6 (10oct) → ---
Whiteboard: [c=progress p= s= u=][p=1] → [c=progress p= s= u=][p(2.1S5)=1]
Depends on: 1074732
Depends on: 1084298
Moving to bug 887198 that has lots of discussions about the behavior.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 887198
You need to log in before you can comment on or make changes to this bug.