From the performance workshop today we know the front-end is wasting a lot of time in reflows that could be avoided by batching. (This was not really a surprise.) I've got a fix that we'll try and land soon, but we will definitely want it uplifted to v1.0.1.

Pointer to Github pull-request

Is the improvement something that we can measure? We don't have any blocking bugs on file for email scrolling performance, so we'll need to loop back with partners to see if they want to take additional risk for an X% win

This is part of our attempt to optimize startup and initial sync. Bug 817095 is the nominal bug for that problem, which is why this bug blocks that bug. We are trying to get performance numbers for startup impact. In answer to your question on https://bugzilla.mozilla.org/show_bug.cgi?id=817095#c52 to someone with better access to the numbers, the numbers I was got forwarded earlier this week show e-mail being ~900ms off the "LA target" on a 512meg device and 2400-3000ms on a 256 meg device. This fix and its friends will be required dependencies of any other startup performance work we do.

Spoke with Andreas - we can do this for 1.1 instead, no more perf fixes needed in email.

(In reply to Alex Keybl [:akeybl] from comment #4) > Spoke with Andreas - we can do this for 1.1 instead, no more perf fixes > needed in email. So we're okay with Qualcomm reporting a 5 second time to start up the e-mail app and display its already stored messages on its 256 meg devices?

Comment on attachment 738572 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/183 This pull request got badly bit-rotted. I'm going to try and clean this up soon, probably with a manual re-application of the patch since the automated merge really just confuses the issue.

Basic Batch working at the moment. Performance numbers using devtools reflow timings. I start reflow counting by first setting up my personal Mozilla inbox account. I log reflow timings from the point I push "continue to mail" until my inbox is listed. All numbers are based on an otoro device. Reflow without modifications: 230.51 ms. 39 Reflow Instances Reflow w/ Batching: 127.45 ms. 36 Reflow instances.

Tagging this bug to put it in the productivity backlog

Measured amount of reflows to login to my Mozilla inbox, which has 757 messages. I measured reflow time from the time I finish adding my account and click 'continue to mail' until all my inbox is downloaded. The amount of reflow spent on a hamachi device is: Before Patch: 218.96 ms After Patch: 104.03

This is awesome. Thank you! Because bug 943498 operates in a similar vein and is more of a candidate for uplift since it has less risk, I don't want to land this until that has landed. I'll try and do at least a first pass review on this soon regardless of whether that bug has landed. Would it be possible for you to also give your patch a try on top of the patch on that bug and see how that impacts the numbers? The interaction of the the two patches will be very interesting!

Here are some new reflow numbers. There was a bug with devtools and gecko, so i had to update gecko which makes the previous numbers invalid. All numbers are with bug943498 applied to the email front end. From the point of adding an account and clicking 'continue to mail': w/ patch: 214.98 ms w/o patch: 312.65 ms From the bottom of my inbox to scrolling to the top and the search bar appears: w/ patch: 45.89 ms w/o patch: 101.69 ms So overall still looks like an improvement. The only thing is that due to the batching, the initial sync seems to be a bit jankier because we update a bunch of stuff, pause, bunch of stuff, pause which was the intended effect. Once everything is synced for the first time, things appear smoother.

Are these results based on a patch rebased for bug 943498 landing? (In reply to Mason Chang [:mchang] from comment #11) > So overall still looks like an improvement. The only thing is that due to > the batching, the initial sync seems to be a bit jankier because we update a > bunch of stuff, pause, bunch of stuff, pause which was the intended effect. > Once everything is synced for the first time, things appear smoother. I think this is concerning and perhaps moving in the wrong direction. We can't sacrifice latency for throughput or overall efficiency. Do we retrigger the jank the next time the user clicks "load more messages"?

(In reply to Ben Kelly [:bkelly] from comment #12) > I think this is concerning and perhaps moving in the wrong direction. We > can't sacrifice latency for throughput or overall efficiency. Do we > retrigger the jank the next time the user clicks "load more messages"? Does turning on APZC reduce our latency concerns? It seems like this change in conjunction with: - APZC - making the (not yet implemented) changes proposed on bug 943498 to greatly reduce our scrollTop touching - the CSS hinting on the scrollable region should let us reduce wasteful paints (I believe :bkelly noted that paint-flashing shows we are doing full painting right now all the time) and re-flows. There will still be the fundamental reflow and paint costs for the DOM nodes we insert or snippets we update, but the overpaint and over-reflow should be dramatically reduced. Having said that, I agree that we should not land this until we're sure we're not regressing the actual perceived performance using the defaults of the v1.4 branch. I do want to avoid this fix bit-rotting again, though, so I'd like to try and get this resolved soon. Mason, what's your procedure for tallying up those frames? Do you have special tooling or are you using stock cleopatra and manual stack frame counting (possibly augmented by filtering?)

(In reply to Andrew Sutherland (:asuth) from comment #13) > Does turning on APZC reduce our latency concerns? > > It seems like this change in conjunction with: > - APZC I am still unsure how this will work in email if the issues with onscroll events are still there. > - making the (not yet implemented) changes proposed on bug 943498 to greatly > reduce our scrollTop touching Might help some, but touching the DOM will cause reflows one way or another. Spreading them out is less efficient in a pure sense, but results in less jank. > - the CSS hinting on the scrollable region > should let us reduce wasteful paints (I believe :bkelly noted that > paint-flashing shows we are doing full painting right now all the time) and > re-flows. There will still be the fundamental reflow and paint costs for > the DOM nodes we insert or snippets we update, but the overpaint and > over-reflow should be dramatically reduced. Benoit told me on IRC that we should not repaint for changes offscreen. That is until APZ lands, and then that behavior is expected. :-\

We landed the will-animate patch on the gaia side which should help a lot with the full screen repainting. When I landed will-animate for email, I didn't see full repaints anymore. As for tallying up reflow counters, you can enable the preference 'devtools.webconsole.filter.csslog'. Once you enable the preference, enter the debug menu through the app manager and you should be able to see how much time is spent in reflows. I dug a bit more, the most perceivable lag comes when syncing and touching the menu to choose which folder to read. This is only on the initial sync from the point of adding an account. Once the account is loaded, it all feels spiffy. Maybe we can choose to batch up a max of 'X' number of elements then shoot of a splice? Most batches still only involve 1 message, but I see some are 10 - 15 messages batched up (which may be from downloading the headers).

Updated to force a flush if we have over 5 pending requests, which should reduce some of the mega batches. I still have to measure reflow counts.

Just did another measurement. By forcing a flush if we have 5 pending requests, our reflow count is 254.77 ms. It also feels a bit more responsive and I can't notice any more considerable lag than without the patch.

Comment on attachment 8347640 [details] https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/277 great work on this conceptually complex patch; I love having the accumulation mode disappear! It looks like there is an ordering issue involving contact resolution, so we'll want a fix for that and a unit test for the suggested new failure case. I also have some comment requests in there. Marking f+, please re-request review when you want me to take another pass.

Blocks a 1.3 CS blocker -> blocking+.

Comment on attachment 8347640 [details] https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/277 Made your changes, let's try this again :)

Hi Mason, Would you be able to please pick up on this 1.3 blocker?

This bug is blocked on my review, which will be coming real soon now.

Comment on attachment 8347640 [details] https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/277 r=asuth with minor review fixes, as discussed. Review fixes are part of the pull request at: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/280 Which I've now landed. Putting the commits in the fixing comment.

landed on gaia-email-libs-and-more/master: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/280 https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/0af073b50733cc4350cc4bf1ff07df010065efb1 landed on gaia/master: https://github.com/mozilla-b2g/gaia/pull/15327 https://github.com/mozilla-b2g/gaia/commit/1c58606fb725112904ab206475ffaaf5158c5881 The automated uplift applies cleanly and is low-risk thanks to low deviation and no changes to the code-base in this area. Thanks very much for all your hard work on this, Mason!

I think this may matter to bug 950733 after all, and timeliness matters, so... Manually uplifted to gaia/v1.3: https://github.com/mozilla-b2g/gaia/pull/15337 https://github.com/mozilla-b2g/gaia/commit/14e199d6a9ad917eacad883820a9f7619dbf42c8

already in tarako branch, remove [tarako] whiteboard