We load a ton of DOM for stuff that isn't used initially (setup, settings, probably some other nodes). We might be able to see a slight improvement if we lazy load DOM nodes which we don't need initially. There are potentially two methods to doing this: 1 - Templates through js files (Similar to what Calendar does) 2 - Commented out sections, similar to what Settings does.

I don't think we want to change-over to JS templating at this time since we've got an idiom and I'd rather switch once to web components when that gets here rather than twice. Does the localization pre-population stuff work with the settings app? And of course, ballpark numbers would be great for this. Since our HTML is only ~1000 lines and our aforementioned idiom has our template nodes live under display: none nodes, we at least shouldn't be paying layout costs.

I will try to get some ballpark numbers today.(In reply to Andrew Sutherland (:asuth) from comment #1) > I don't think we want to change-over to JS templating at this time since > we've got an idiom and I'd rather switch once to web components when that > gets here rather than twice. > > Does the localization pre-population stuff work with the settings app? > > And of course, ballpark numbers would be great for this. Since our HTML is > only ~1000 lines and our aforementioned idiom has our template nodes live > under display: none nodes, we at least shouldn't be paying layout costs. I will profile and get some performance numbers so we can decide if it's worth switching over earlier. The localization stuff should continue to work if we follow what settings has done.

So I commented out most of the elements that were not accessed during startup, and the gains appear to be minimal (20-30ms or so from my testing). I really dislike what it does to the HTML doc, so I think it's best if we skip that for now. I'm still going to use this bug to track improvements to the initial work that we do to load up the templates.

Thanks for running the numbers; I agree that commenting out swathes of HTML without a huge payoff is undesirable. It sounds like you're talking about deferring some of our processing of the nodes, which does seem like it could save us some cycles without getting too ugly.

(In reply to Kevin Grandon from comment #3) > So I commented out most of the elements that were not accessed during > startup, and the gains appear to be minimal (20-30ms or so from my testing). > I really dislike what it does to the HTML doc, so I think it's best if we > skip that for now. I'm still going to use this bug to track improvements to > the initial work that we do to load up the templates. That seems weird that it save only 20-30ms. On Settings which was 2000 lines of code at the time where this stuff has been done it was taking 1s for the parser to read it - display: none saves layout but it was still taking forever to parse the doc. So I would have expect a better gain here. Could it be that something else hides the win?

Taking 1sec to parse 2000 lines of HTML is nuts though; did anyone actually use a profiler with valid native backtraces to figure out what was actually slow about that? (ex: with --disable-elf-hack, although things might have been otherwise broken at that time anyways.) Or check with Henri Sivonen?

(In reply to Andrew Sutherland (:asuth) from comment #6) > Taking 1sec to parse 2000 lines of HTML is nuts though; did anyone actually > use a profiler with valid native backtraces to figure out what was actually > slow about that? (ex: with --disable-elf-hack, although things might have > been otherwise broken at that time anyways.) Or check with Henri Sivonen? The profiler was barely running at that time and since we were in the rush we did not opened a bug :(

Also, I would not wait on improvements in the HTML parser to do something here.

This is a halfway patch which provides an interface so we can lazy-load the DOM nodes if we wish to. There also should be slight performance gains as it will be doing less work at startup. (Not querying for unused elements for example)

This looks very reasonable. My main concern is that the lookup mechanism being dependent on a function call loses us the potential of the shape lookup being cached at each usage location. It was speculative, but that's why I structured it that way. Since our implementation is already file-centric based on card family for on-demand JS/CSS loading, and the DOM tree is also sliced in that fashion, it seems like we could accomplish on-demand DOM loading by throwing that in there too. Even now, our setup cards could require 'dom/sup' which would trigger: "supNodes = processTemplNodes('sup');" But all that really matters is the startup number to usability (including message list population), so we probably want to add that performance event that was requested in conjunction with the always-refresh patch that should land soon, then we can run with this and check what it does to the number and take it or not. If the startup aspect improves but the overall impact's not great, we can try on-demand loading with processTemplNodes instead.

I've update the pull request to include full DOM lazy loading as an experiment, and attached two profiles. One with the current state of master, and one after this pull request. Most of the heavy lifting gets shifted over to the left by about ~70 ticks, I'm not sure what this means for the overall performance picture and will continue to look.

Gabriele - Here are two profiles I'm currently experimenting with. The latter is the result of some work I'm doing to lazy load the DOM nodes. I'm just trying to decipher this as adding manual timing hooks to the email UI is more complex than calendar, and results widely vary from load to load. I'm wondering: A - Am I taking the profile correctly? B - Do you see much improvement in the second profile? Thanks! Current state of email: http://people.mozilla.com/~bgirard/cleopatra/#report=994de37dcd88065018dd66b2259b9824b8d9e778 Email with DOM lazy loading: http://people.mozilla.com/~bgirard/cleopatra/#report=86d29e13a47bd7fae38d938cfed01d44cd6aabb0

(In reply to Kevin Grandon from comment #14) > A - Am I taking the profile correctly? Yes, the profiles look fine. However I would suggest to wait a bit before launching the applicaiton when taking a profile so that the prelaunched process can settle down before starting to work again. This adds a long stretch before the app launches where only the nsAppShell::ProcessNextNativeEvent::Wait() can be seen and makes it easy to spot where the application is starting. In your second profile I have a hard time spotting where the application is starting so I can't measure startup time very precisely. > B - Do you see much improvement in the second profile? I think there's a drop but since I can't really tell apart the startup time in the second profile I cannot be sure. Can you take another profile making sure that you wait 10-15 seconds with the phone idle before starting the app? Additionally I see that there's now 3 distinct sync reflows that are being triggered by JS code during startup: - once in _hideSearchBoxByScrolling() which we already had discussed in the parent bug - once again in onMessagesSplice(), looking at the code there's many places where this could be triggered (many lookups to .offsetTop, .clientHeight, etc....) - the last one is caused by onScroll() being called by onSliceRequestComplete() Ideally we should have only one reflow during startup when everything is ready on the page however I'm not sure if it's possible to achieve that effect here.

Patch no longer applies, and it appears we don't really want to go down this path. Closing for now.

Startup time still matters to us, and we've taken care of most of the other low hanging fruit, so this is still interesting, so no need to wontfix. James Burke got some numbers recently, available at: https://gist.github.com/jrburke/70363a7bd77ef92ddeb5 Key point was a 55ms improvement which is meaningful in absolute terms. James is looking at the performance atom stuff too, so especially once we have that, it should be much easier to see the win on this.

Pointer to Github pull-request

Comment on attachment 738913 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9258 Picks up @KevinGrandon's original lazy DOM approach, but updated for latest master. Also removes dynamic loader used for back end code since that non-worker code is now just a single file. Combining these changes for expediency and ease of uplift. Related GELAM pull for the GELAM code: mozilla-b2g/gaia-email-libs-and-more#184

Comment on attachment 738913 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9258 This patch is obsolete now, some of it landed, will need to redo the main part of the patch that does the actual lazy DOM loading bit.

This is a rebased version of my original one against latest master.

Running performance test against today's master build, I'm seeing similar gains. With lazy dom nodes: "mozPerfDurationsAverage": 930.6 Without lazy dom nodes: "mozPerfDurationsAverage": 982.4

We're planning to wait on the lazy loading until we've got our datazilla numbers in the tree for actual time-to-messages. Currently leaning towards jrburke's alternate implementation that hosts the lazy HTML in separate files rather than commented out HTML (but which can get inlined into the JS as strings for performance reasons). We'll evaluate the various approaches at that time, which should be in the next few days.

That sounds like a good approach to consider. We actually do something similar in Calendar where we lazy load templates when needed. I would suggest that it might be worthwhile to look at abstracting the calendar code we have and move it to shared/.

Comment on attachment 744895 [details] Github pull request pointer We're going with :jrburke's more aggressive re-factoring of the front-end to load HTML from separate files and persist the HTML structure to cookies (rather than persisting database state).

Sounds good, looking forward to that approach! I'm going to close this out as I assume you have a bug for that?

(In reply to Kevin Grandon :kgrandon from comment #26) > I'm going to close this out as I assume you have a bug for that? Kevin, Let's make sure we have a bug for James' work before closing this. James and Andrew, Please dup this bug against the one being used to track James' work.

Right, thanks for the reminder, just created a bug that summarized the results from the email mini-work week in 886446.