Sam Foster [:sfoster] (he/him) Assignee
	Description • 11 years ago

The richgrid binding's mutator methods recently got a 'skipArrange' param, which is not used or not used effectively in some of the views with grids. This results in repeated arrangeItems calls. As each call can involve quite a bit of DOM interaction, fixing this should buy some tangible performance gains. For example, I recently logged the following during startup:

richgrid#start-bookmarks-grid: 9 calls
richgrid#start-history-grid: 17 calls
richgrid#start-topsites-grid: 2 calls
richgrid#snapped-topsites-grid: 1 call
richgrid#start-bookmarks-grid: 1 call
richgrid#results-richgrid: 2 calls
richgrid#start-remotetabs-grid: 1 call
richgrid#snapped-topsites-grid: 2 calls
richgrid#searches-richgrid: 24 calls

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 1 • 11 years ago

Make use of the skipArrange params to significantly reduce the number of times we rebuild the richgrid / items - at startup and during use. New numbers from startup with this patch: 

richgrid#start-bookmarks-grid: calls: 2
richgrid#start-history-grid: calls: 2
richgrid#start-topsites-grid: calls: 2
richgrid#snapped-topsites-grid: calls: 3
richgrid#results-richgrid: calls: 2
richgrid#start-remotetabs-grid: calls: 11
richgrid#searches-richgrid: calls: 3

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 2 • 11 years ago

Ack, that's richgrid#start-remotetabs-grid: calls: 1 not 11

	Matt Brubeck (:mbrubeck)
	Comment 3 • 11 years ago

Comment on attachment 755421 [details] [diff] [review]
Reduce the number of calls to richgrid arrangeItems

Review of attachment 755421 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/history.js
@@ +30,5 @@
>    },
>  
>    populateGrid: function populateGrid(aRefresh) {
> +    let inBatch = this._inBatch;
> +    this._inBatch = true; // always batch up grid updates to avoid redundant arrangeItems calls

Another way to do this is initialize _inBatch to 0, then use _inBatch++ to set it and _inBatch-- to unset.  This is a bit more robust for things like nested calls.  We don't seem to require nesting here, but feel free to switch to this style if you like.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 4 • 11 years ago

I updated the patch to use the inBatch++ / -- style for the nesting case in history.js. Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/d448b2c7e12b

	Ryan VanderMeulen [:RyanVM]
	Comment 5 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/d448b2c7e12b