Reduce layout thrashing by optimizing dxSTable row creation

[stickz]

This resource explains the issue in more detail. ruTorrent was taking 20s to load with 5000 torrents, when I refreshed the page. I ran a profile and found this function was thrashing the layout. After implementing this fix, my loading time was down to 5s.

Benchmarks for theWebUI.addTorrents()
Loading times with 500 torrents are reduced from 800ms to 250ms. Approximately 150ms is a performance penalty.
Loading times with 5000 torrents are reduced from 18s to 1s. Approximately 400ms is a performance penalty.

There is a performance penalty for this fix. theWebUI.loadTorrents() must be run afterwards to display the 20 to 30 rows of torrents that will be initially visible to the user. This can no longer be done during theWebUI.addTorrents(). However, as demonstrated above, the page loading speed benefit is greater than the performance penalty, in virtually every situation. The user may see approximately 100ms longer loading times with only 50 torrents. This is relatively small and trivial though.

[Novik]

Thanks, but IMHO instead this patch we need to optimize function dxSTable.prototype.getMaxRows (for example, remove condition 'this.viewRows<this.maxViewRows').
Now we haven't ascient IE, Safari etc.
I will check it in later.

[stickz]

Do you think it's a good idea to rewrite this function though? There are other methods which call dxSTable.prototype.getMaxRows. This could have unintended side effects when calling them.

I investigated your suggestion further and found that dxSTable.prototype.refreshRows also calls this function afterwards, to correct the hack to the reflow issue I created. That is not going to work properly, without further complicating the code.

[stickz]

@Novik Could you review this pull request again? I optimized dxSTable row creation and removed an unnecessary table sort. The results are even more astonishing than before and the code quality is much higher.

Benchmarks for theWebUI.addTorrents()
Loading times with 500 torrents are reduced from 800ms to 175ms. Approximately 70ms is a performance penalty.
Loading times with 5000 torrents are reduced from 18s to 520ms. Approximately 70ms is a performance penalty.

The performance penalty is now reduced to approximately 70ms, regardless of the number of torrents.

[Novik]

Loading times with 5000 torrents are reduced from 18s to 520ms.

And this is not surprising, because really you simple remove DOM filling at the first call of function. I.e. if we will have only one call of addTorrents(), then user will see nothing.
I think in the such way you may reduce timings yet more - if you write something like if(this.firstLoad) { this.show(); return; } at the start of function...
Can you explain your changes, please?

[stickz]

And this is not surprising, because really you simple remove DOM filling at the first call of function. I.e. if we will have only one call of addTorrents(), then user will see nothing.

loadTorrents() is called inside the addTorrents() function. This goes through and fills the DOM after dxSTable rowIDs Array is populated. This improves the flow so the code doesn't thrash the layout. We still have to populate this array to fill the DOM.

[Novik]

This goes through and fills the DOM after dxSTable rowIDs Array is populated.

Yes, you are right, through the sequence loadTorrents -> show -> resize -> resizeTop -> dxSTable.resize -> dxSTable.refreshRows.

But yet one moment. Pay attention to this fragment, please:

        if (!fast)
	{
             ....
	}
	else
	{
             ....
	}

	var self = this;
	if((this.sIndex !=- 1) && !this.noSort)
		this.sortTimeout = window.setTimeout(function() { self.Sort(); }, 200);

We may have here sIndex !=- 1 (if user press on the column header in the previous session, for example).
As result, we will have call of Sort. We really need this in the "fast" case?

[stickz]

We really need this in the "fast" case?

Good catch. I just removed the table sort from the "fast" case. It's only taking about 300ms now to add the torrents, regardless of the number of torrents added to the web client. I think this PR is ready for merge now.