Closed Bug 835507 Opened 12 years ago Closed 12 years ago

[Gaia::E-Mail] Consider lazy loading/delaying resources

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::E-Mail
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

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

Status:
RESOLVED FIXED
Project Flags:
blocking-b2g tef+
Tracking Flags:
Tracking Status
b2g18 --- fixed
b2g18-v1.0.1 --- fixed

People

(Reporter: kgrandon, Assigned: kgrandon)

References

Details

(Whiteboard: [FFOS_perf] QARegressExclude [qa-])

Attachments

(1 file)

Github pull request pointer
223 bytes, text/html
asuth
: review+
Details
I've noticed about a 200ms speedup if we remove all unneeded resources from the initial page load. It might be worthwhile to lazy load/delay resources that are not needed for the initial view.
Assignee: kgrandon → squibblyflabbetydoo
Oops, let's swap this one (Email) and Bug #836365 (Dialer).
Assignee: squibblyflabbetydoo → kgrandon
Feedback on: https://github.com/KevinGrandon/gaia/compare/bug_835507_lazy_email The lazy loading seems pretty reasonable. Numbers would indeed be great. Thoughts, recognizing that this was an initial pass and not intended to be a final review: - I like your choice of things to preload; you clearly took care to avoid adding latency to the load path. - I'm concerned about increasing the latency of instances where user activity triggers loads, particularly since dynamically loading stylesheets can require the whole DOM tree to be re-evaluated. I think it would be good if we could have an App.loader.preload() method or some such that we could throw some timeouts at that would go through a list of things and trigger their loads. In our dreams we might have something that would look for idle periods as told to us by the back-end to find a good time for us to do this. If you're on board with this, let's plan to do that assuming the general numbers are good. If you think it might not work, I'd love it if you could get a profile with event timestamps around a click triggering such a load so we can see the numbers. - the path_segment[0] => dynamic lookup bit really wants a comment in the load() method signature and in the HTML comment which lists what we plan to lazy load (which is great to have)
Thanks for the feedback. I agree that having some kind of method to load the remaining resources would be good. I wonder if there is some external heuristic we could use, perhaps checking for intervals of MozAfterPaint. E.g., once we stop receiving paint events for 500ms, preload other files. I have no idea if something like that would work, just throwing it out there.
MozAfterPaint seems like a great idea.
(Content can't listen to that, unfortunately.)
Chris - are you sure about that? I was able to implement this quickly and had no problems listening to MozAfterPaint: https://github.com/KevinGrandon/gaia/commit/a2419e94dd34038f5f72fe5360aa805f5ac67321 Perhaps my environment is not configured correctly?
Also I ran some performance tests for email on my unagi with one account. About 20 runs from page <head> until the _init method in mail-app.js. Master branch: 4,036ms average Master branch: 3,760ms average Lazy loading these script/stylesheets should buy us 200-300ms. I'm trying to see what else we can lazy load.
We accidentally left mozafterpaint enabled for gaia (with a gecko pref override) but have fixed that problem. Please don't use it even if it happens to work on the commit your branch is based on.
Attachment #709840 - Flags: review?(bugmail)
Comment on attachment 709840 [details] Github pull request pointer r=asuth with noted requests that I think you've addressed now (thanks!). Manual runs seem to show nothing is broken. I also ran a profile on a gecko build with source saving disabled with this patch applied and can confirm that the deferred loading pushed out the loads, which took ~17 ticks when they eventually ran, and they were fairly clustered. I am unable to upload the profile for some reason. ("Error 0 occurred uploading your file.")
Attachment #709840 - Flags: review?(bugmail) → review+
Landed: https://github.com/mozilla-b2g/gaia/commit/886b27d9d77f7c3a6fd05631e45cd63e8bbe69e9
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [FFOS_perf]
My empirical tests show that this patch helps saving 100ms during load. Plus, Bug 841251 that we really need both for v1.0.1 and v1-train depends on this (because this bug actually layed out the lazy loading logic for the email app). Plus, this applies cleanly on v1-train and v1.0.1 and my manual tests show that everything seems to work.
blocking-b2g: --- → tef?
tracking-b2g18: --- → ?
(little nervous about tef+ing this, but email app start is one of the big areas of focus still and 100ms is just too tempting to say no to)
blocking-b2g: tef? → tef+
v1-train: f6ecea2a9ebf9e946eda38928df8cb802209a26b
status-b2g18: --- → fixed
status-b2g18-v1.0.1: --- → affected
v1.0.1: eb8c496766eafd834da97530bfd47d8b0347004a
status-b2g18-v1.0.1: affected → fixed
tracking-b2g18: --- → ?
does not make sense to create a regression issue.
Flags: in-moztrap-
Can you please provide steps to verify this fix - as we will blackbox test from the UI?
This is just an implementation issue; any regressions new failures would likely just be filed as new bugs and would reference this bug as the source of the regression when investigated by developers working those regressions.
Whiteboard: [FFOS_perf] → [FFOS_perf] QARegressExclude
Whiteboard: [FFOS_perf] QARegressExclude → [FFOS_perf] QARegressExclude [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: