[Calendar] Lazy load dom nodes

RESOLVED FIXED

Status

Firefox OS
Gaia::Calendar
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

({perf})

unspecified
x86
Mac OS X

Firefox Tracking Flags

(blocking-b2g:-, b2g18 fixed, b2g18-v1.0.1 wontfix)

Details

(Whiteboard: FFOS_perf)

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
I profiled and it appears that we can shave off about 400ms if we lazy load all dom nodes which we don't need at startup.

From the top of the <head> to the init() method it usually takes about 1,400ms, this goes down to about 1,000 if I get rid of everything not needed on the initial page.
(Assignee)

Updated

5 years ago
Keywords: perf
Whiteboard: FFOS_perf
(Assignee)

Comment 1

5 years ago
400ms seems like a *lot* of time to be spending in the ~300 or so lines of Calendar DOM..
(Assignee)

Comment 2

5 years ago
Created attachment 714736 [details]
Calendar Profile (master)
(Assignee)

Comment 3

5 years ago
Created attachment 714737 [details]
Calendar Profile (lazy loaded DOM)
(Assignee)

Comment 4

5 years ago
Created attachment 714820 [details]
Github pull request pointer
(Assignee)

Comment 5

5 years ago
Comment on attachment 714820 [details]
Github pull request pointer

Hi David - as James is out, I'm looking for a review of this one if you have time. If you think someone else might be a better reviewer for this, let me know. Thanks!
Attachment #714820 - Flags: review?(dflanagan)
(Assignee)

Comment 6

5 years ago
Comment on attachment 714820 [details]
Github pull request pointer

Kaze - Also adding you as a reviewer for this one if you have time. This is similar to what we've done with settings, so you might have some insights into it.
Attachment #714820 - Flags: review?(kaze)
Comment on attachment 714820 [details]
Github pull request pointer

Kaze is on pto and we did the Settings experimentation together so I feel confident to r+ this patch. Let's land it and let calendar join the other apps :)
Attachment #714820 - Flags: review?(kaze)
Attachment #714820 - Flags: review?(dflanagan)
Attachment #714820 - Flags: review+
(Assignee)

Comment 8

5 years ago
Thanks vivien, landed here: https://github.com/mozilla-b2g/gaia/commit/2818fd2c6efa8533e23080b265f56d4cad034b85

The only thing that might be nice to have is full-lazy loaded support instead of loading all up front - but I don't have bandwidth to do that right now.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

5 years ago
There is unfortunately an additional pull request that was just landed for this one: https://github.com/mozilla-b2g/gaia/commit/a424b9ed0d417c05f49b44c5afff0a8aff8bebf2

This pull request translates the dom nodes (noticed it was broken while testing).
(Assignee)

Comment 10

5 years ago
Comment on attachment 714820 [details]
Github pull request pointer

I am sure partners will want to see calendar performance improvements for V1, so asking for approval-gaia-v1.

User impact if declined: Slow calendar.
Testing completed: Manual testing of most features that would be impacted.
Risk to taking this patch (and alternatives if risky): No risks as far as I can tell.
String or UUID changes made by this patch: (Two prior pull requests)
Attachment #714820 - Flags: approval-gaia-v1?(21)
Sorry I didn't get to this review in time Kevin... I got asked to review a surprising (for me) number of patches over the weekend.  Thanks for reviewing Vivien!
Comment on attachment 714820 [details]
Github pull request pointer

Low risk (this trick is used in a few other apps already), big win (hundreds of ms). Let's land tht to v1.0.1 / v1-train.
Attachment #714820 - Flags: approval-gaia-v1?(21) → approval-gaia-v1+
v1-train@7b26840429517006548889d1ba4c68da2e02f539
status-b2g18: --- → fixed
(Assignee)

Comment 14

5 years ago
The translation fix has also been cherry-picked into v1@cee48b75cfa8b9bd63dfb15b5b8f64a2e858634e.
Comment 12 from Vivien talked about v1.0.1. Should we uplift this or do we think the calendar startup time is good enough for v1.0.1 ?
blocking-b2g: --- → tef?
(tef-, calendar start time is not on the v1.0.1 must-fix radar)
blocking-b2g: tef? → -
1.0.1 wontfix per comment 16
status-b2g18-v1.0.1: --- → wontfix
You need to log in before you can comment on or make changes to this bug.