Closed Bug 863671 Opened 11 years ago Closed 11 years ago

Keyboard latency increase caused by asynchronizing mozKeyboard event send

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: jld, Assigned: hub)

References

Details

(Keywords: perf, Whiteboard: c= ,)

The fix for bug 858454 deferred the keyboard's sending events to the app to a separate event instead of running it synchronously with the keyboard's script.

However, repainting caused by the running of script that sent the keys is run from a timer by the nsRefreshDriver, and this can (and, empirically, often does) be run before the key event sending callback.

So, instead of blocking the main thread event loop for an additional ~10ms during the touch event handling and letting the app repaint in parallel with the keyboard, we add up to 50-60ms (which will decrease to 30-40ms soon, after the style optimizations land) on the critical path of getting the user's keypress painted in the app.  And it's nondeterministic whether or not this happens.  (But also, "up to" because, on a single-core phone, the kernel may give some time to the main process's keyboard repainting while the app repaints.)

This might be a problem, so it should have a bug.
Whiteboard: c=performance
Assignee: nobody → hub
Status: NEW → ASSIGNED
Keywords: perf
I don't know if this makes much difference now — possibly due to the keyboard repaint being fast enough to finish before the app's repaint, or something like that.  Here's a profile with the change in question reverted:

https://people.mozilla.com/~bgirard/cleopatra/#report=bdbb8d949776e6a58a415611f00800df230486e0
From my investigation we don't repaint the keyboard at each keypresses. It is only done when you press modifiers like '?123' or shift.
BTW I can't view that profiler data. And which hardware did you run it on.
Try http://people.mozilla.org/~bgirard/cleopatra/#report=bdbb8d949776e6a58a415611f00800df230486e0 — it looks like the profile data is blocked as mixed content if the UI is loaded over https.

But also, don't use that profile — I've just discovered that that build had IonMonkey disabled due to a branching accident.  Revision 824abf58d of my b2g-manifests fork should fix that.
I don't seen difference between one and the other. At least not user visible.

Profiles seems to looks similar.
(In reply to Hubert Figuiere [:hub] from comment #2)
> From my investigation we don't repaint the keyboard at each keypresses. It
> is only done when you press modifiers like '?123' or shift.

Repaint as in PresShell::Paint — render the DOM into pixels, wake up the compositor thread, etc.  I agree that we're not changing the markup (except for classLists) unless there's a case/mode/whatever change.
Yeah that's the repaint I was looking at afterwards anyway.
Seems to not be an issue. We have had other perf changes in the keyboard lately.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Whiteboard: c=performance → c= ,
You need to log in before you can comment on or make changes to this bug.