With John Daggett's help this evening I was able to identify the source of some of the extra reflows in contacts scrolling. Specifically, why we are regenerating text runs while scrolling contacts that have already been rendered. It appears to be this code in list.js: var updateSingleRowSelection = function updateSingleRowSelection(row, id) { var id = id || row.dataset.uuid; var check = row.querySelector('input[value="' + id + '"]'); if (!check) { return; } check.checked = !!selectedContacts[id]; }; This unconditionally sets the value of the checkbox to the backing selection state of the contact model. Most of the time this will be setting the checkbox to the same value it already contains. Unfortunately this seems to trigger a reflow and the regeneration of a bunch of textruns. Talking to Daniel Holbert this appears to be a platform bug. I'm going to propose a gaia work around here and then open another bug for the platform issue.

This avoids the additional reflows when scrolling content that is already rendered.

Profile of contacts after the change: http://people.mozilla.org/~bgirard/cleopatra/#report=86e9331c1bddd2b30df35f26e4953411f9fbd639 Profile before the change, but I'm not sure I got enough history: http://people.mozilla.org/~bgirard/cleopatra/#report=33b715fb04933dae2e5f39cc4d38810e1a991244

Looks good, there's a lot fewer reflows showing up in the newer profile.

Comment on attachment 8388919 [details] Pull request at https://github.com/mozilla-b2g/gaia/pull/17046 LGTM, thanks for catching that one! Do you think we could add unit tests to the PR?

I think I could add some asserts to the existing unit tests to validate the checked attribute is set correctly. I don't think I can validate the (lack of) reflows very easily, though.

Will be more than enough just checking that we don't change the checked attribute. By the way, not for doing it here, but just found a place where Etienne is using integration tests to detect that: https://github.com/etiennesegonzac/gaia/blob/e5248fadbc2fbb58fc4e00b1a35b86adbf0062c8/tests/js-marionette/reflow_helper.js https://github.com/etiennesegonzac/gaia/blob/e5248fadbc2fbb58fc4e00b1a35b86adbf0062c8/apps/communications/dialer/test/marionette/keypad_test.js Just to remind my self for future tests :)

Hmm, it appears changing the checked assignment may have been a false positive. If I keep scrolling I eventually see the re-created textruns again. For example: 1) make reference-workload-medium 2) scroll to bottom of the list to get all items rendered... lots of textruns while we scroll 3) scroll back to start of list... about half way textruns start getting created again With reflow logging enabled I don't see any reflows. I also don't see any textperf-reflow NSPR log messages. Just the textrun logs. So the checkbox thing is probably a red herring. John, should I expect the textruns to get re-created in this case?

It appears text runs get expired every 10 seconds in FrameTextRunCache. So this is normal and expected.

Closing this as invalid. Note, I think overall we are in better shape with contacts painting text now that tiling has landed. We no longer need to repaint huge portions of the list, and remaking their text runs, in one go.

(In reply to Ben Kelly [:bkelly] from comment #7) > Hmm, it appears changing the checked assignment may have been a false > positive. If I keep scrolling I eventually see the re-created textruns > again. That's true but are you sure the change in state doesn't cause textruns to be dumped. I guess the question is if you scroll for less than 10 seconds with and without the patch here, do you see the same number of textruns built or less after the patch?

(In reply to Ben Kelly [:bkelly] from comment #7) > Hmm, it appears changing the checked assignment may have been a false > positive. If I keep scrolling I eventually see the re-created textruns > again. > > For example: > > 1) make reference-workload-medium > 2) scroll to bottom of the list to get all items rendered... lots of > textruns while we scroll > 3) scroll back to start of list... about half way textruns start getting > created again > > With reflow logging enabled I don't see any reflows. I also don't see any > textperf-reflow NSPR log messages. Just the textrun logs. > > So the checkbox thing is probably a red herring. > > John, should I expect the textruns to get re-created in this case? Yes, because textruns might be rebuilt during the paint cycle rather than because of a reflow. The comparison should be the number of textruns before/after the checkbox change, assuming you can test in an equivalent manner.