Closed Bug 891029 Opened 7 years ago Closed 4 years ago

[sms] Do less synchronous reflows for the compose input height management

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(tracking-b2g:backlog)

RESOLVED WORKSFORME
tracking-b2g backlog

People

(Reporter: julienw, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [c=effect p= s= u=])

Attachments

(1 file)

In the 2 methods "updateInputHeight" and "setInputMaxHeight" we're doing lots of calculations, and some of them are (probably) triggering synchronous reflows.

This bug is to investigate if we're really doing some and to fix them if possible, possibly inserting some geneated CSS style sheets.

The user-facing effect is an effect of lag while he's typing a SMS, or when he's displaying/hiding the keyboard, which is extremely unpleasant.
Depends on: 902500
ni? Julien. please renom after further investigation on how much impact it is to the end users. Thanks
blocking-b2g: koi? → ---
Flags: needinfo?(felash)
You should try writing a SMS with a loaded thread (try one of the reference workload, even the light one) to know the user impact. You can also talk with Alexandre Lissy ;)

It's _very_ unpleasant.

The reflow time when showing/hiding the keyboard is also very long, and profiling with Gabriele we saw there were 3 synchronous workflows. This is most certainly avoidable.

We already started some work (see bug 902500) and we may nominate individual smaller bugs instead of this one if you do prefer.
blocking-b2g: --- → koi?
Flags: needinfo?(felash)
blocking-b2g: koi? → koi+
Whiteboard: [u=commsapps-user c=messaging p=0]
Julien and Gabriele, could you please share the profiling you've performed ?
Flags: needinfo?(gsvelto)
Flags: needinfo?(felash)
The profiling we once did is no longer relevant.

So, here are the various cases that should be profiled:
* what happens when we press the first letter in the composer
* what happens when we press the second letter
* what happens when we press the letter that makes the input grows
* what happens when we open the keyboard
* what happens when we close the keyboard

I think the most important issues are with the last 3 cases now, when we actually change measurements.
Flags: needinfo?(gsvelto)
Flags: needinfo?(felash)
Attached file sms_profiling.zip
Please find a set of profiling data that should cover the proposed cases.
Hm sorry, covered cases should be: opening the keyboard, closing the keyboard and growing input.
Alexandre> could you try to analyze them ? Or would you like Gabriele's help ? :)
When loading some of the Messages.sym files in Cleopatra, I see sync reflows, is it what we are trying to kill?
(In reply to Julien Wajsberg [:julienw] from comment #7)
> Alexandre> could you try to analyze them ? Or would you like Gabriele's help
> ? :)

I think Gabriele could help me understanding more precisely yes, if he has time :)
(In reply to Alexandre LISSY :gerard-majax (off 26/08-08/09) from comment #9)
> I think Gabriele could help me understanding more precisely yes, if he has
> time :)

I've got a last patch to finish and then I'll come over to help :)
This is not a release blocker, we already did some of it for 1.2, let's move the rest to 1.3.
blocking-b2g: koi+ → 1.3?
Not blocking but we would definitely take a patch
minused per comment 12
blocking-b2g: 1.3? → -
Depends on: 949457
Keywords: perf
blocking-b2g: - → backlog
Julien, is this still an issue?
Flags: needinfo?(felash)
Priority: -- → P4
Whiteboard: [u=commsapps-user c=messaging p=0] → [c=effect p= s= u=]
I think bug 949457 fixed this now but I'd like a profile before closing it.
Flags: needinfo?(felash)
blocking-b2g: backlog → ---
Summary: [sms] Do less synchronous workflows for the compose input height management → [sms] Do less synchronous reflows for the compose input height management
The reflows (layout + paint especially) are not synchronous anymore.

They're still quite costly though: layout takes 50ms and paint 30ms on my workload, which is expected because it happens in a big thread.
In the "New message" window, so with a much smaller DOM, layout is a lot shorter (15ms) but paint is still quite long (can take ~27ms). I guess this is costly because we're painting the whole contenteditable at each keypress... 
I'll file a separate bug with a smaller testcase.
Flags: needinfo?(felash)
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
added a testcase in bug 972238.
Flags: needinfo?(felash)
You need to log in before you can comment on or make changes to this bug.