Synchronous reflow in showing the keyboard

RESOLVED FIXED

Status

Firefox OS
Performance
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: cyu, Assigned: rudyl)

Tracking

unspecified
x86_64
Linux

Firefox Tracking Flags

(b2g-master fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
This profile
http://people.mozilla.org/~bgirard/cleopatra/#report=96bc2ae5817026e83da6684cfc9ed1c3168a5ad5&filter=[{%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A30520,%22end%22%3A30834}]&selection=0,243,4,74,6,319,320,6,321,322,6,321,323
shows shows ~150 ms is spent in synchronous reflow in LayoutPageView.getVisualData() (layout_page_view.js). We need to optimize this function like caching computed results, pre-calculation, or applying some heuristics.
Created attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

This is a simple patch to cache the visual data in layout page view, so that it won't calculate it again when you show the keyboard repeatedly.

Cervantes,

Could you please help give some feedback on if this could improve the response time of keyboard show-up?
Thanks.
Attachment #8572499 - Flags: feedback?(cyu)
(Reporter)

Comment 3

3 years ago
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

I tested this patch and it greatly reduces the time spent in getvisualData().
Attachment #8572499 - Flags: feedback?(cyu) → feedback+
(Reporter)

Comment 4

3 years ago
(In reply to Cervantes Yu from comment #3)
> Comment on attachment 8572499 [details] [review]
> [gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master
> 
> I tested this patch and it greatly reduces the time spent in getvisualData().

Even for the first launch, which should miss the cache (although I don't know why) :).
Assignee: nobody → rlu
Status: NEW → ASSIGNED
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

Tim, could you please help review this change?
Thanks.


Cervantes,

Thanks for verifying this patch for performance improvement.
Attachment #8572499 - Flags: review?(timdream)
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

Please read comment 4. I am not sure the LayoutPageView#totalWidth is there when LayoutPageView#getVisualData() is called for the first time. How about fill the value from LayoutPageView#options.totalWidth? What's the difference between two?
Attachment #8572499 - Flags: review?(timdream)
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

Patch updated per your advice.

For comment 4, actually I don't know why this could get rid of the sync reflow for the first time the keyboard is shown after a discussion with Cervantes.

Maybe this is because we push back the timing of calculating the visual data a little bit, and this somehow avoid to cause anther reflow.
Attachment #8572499 - Flags: review?(timdream)
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

Need test cases for getVisualData()
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

Please set the review flag again when the tests are added.
Attachment #8572499 - Flags: review?(timdream)
Comment on attachment 8572499 [details] [review]
[gaia] RudyLu:keyboard/Bug1131969-cache_visual_data > mozilla-b2g:master

Test cases added, please let me know if there is any other case I did not think of.

Thanks.
Attachment #8572499 - Flags: review?(timdream)
Attachment #8572499 - Flags: review?(timdream) → review+
Landed to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/0567a8a885ee924e54ad485b3e9f76a0436132d3
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-b2g-master: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.