twlikol / GridViewScroll

Freeze column and fixed header in Table or GridView
MIT License
115 stars 43 forks source link

Poor performance due to repeated reflows #68

Open ExpHP opened 4 years ago

ExpHP commented 4 years ago

This design of this library appears to try to handle one part of the output at a time (e.g. the header, the body, the frozen columns), but this leads to it alternating between modifying the DOM and reading its computed layout. Not only that, but even just while it is building the header or the frozen columns, it is reflowing on every cell. In a table with a mere 175 rows, I found that appendFreezeContent takes an entire 1.2 seconds.

One possible fix for appendFreezeContent is pretty simple: Create all of the "helper" wrappers before the loop. In my local copy I have also incorporated mbrucco's changes which I need, so I can't easily make a PR, but here is the basic idea:

https://github.com/exphp-forks/GridViewScroll/commit/e0aed0136ef4dfc4565b5a8c35019b74e50cdde1

Even with that fix, it's still too slow for resizing:

gridview-perf

Looking at the performance shows three big remaining problems:

image

  1. shows that calculateHeader has a loop that can probably be similarly optimized.
  2. shows that appendFreezeContent is still heavily affected by changes in the previous steps. It will be necessary to batch together all reads and all writes separately.
  3. shows that to get best performance it will be necessary to find a way to trigger a resize without undo. Deleting and recreating all of those DOM elements simply requires too much layout to be recomputed.

It looks too difficult to fix problems 2 and 3 in the existing code so for now, in my own project I am probably going to try to rewrite the tool (which I suppose will also help me see how it works). But hopefully some improvements can find their way upstream at some point...

ExpHP commented 4 years ago

My rewrite is more or less complete (in some prototypical form), and allows for super fast resizing. I am a bit weary right now to make a repository for it or be burdened to maintain it in any substantial manner, so for now I am going to simply post a gist of it to at least provide something for others who encounter this issue:

GIF image: Demo of GridViewScroll performance rewrite ![gridview-fast](https://user-images.githubusercontent.com/1411280/91379613-142f4c00-e7f1-11ea-9aac-5bf3cc5de663.gif) Note: The flickering in cell backgrounds as I resize it are GIF artefacts. ---

https://gist.github.com/ExpHP/db039dac0ba35bb7a3a49b0321700fcc

In this rewrite, you are meant to put the table in a resizable div (which should have overflow: hidden) and supply that container's id as containerId to GridViewScroll. Call enhance() once, then call .maxContainerSize() and use that to set min-width and max-width on the container. Then use something like ResizeObserver to call .resize() (not .enhance()) whenever the container size changes.

// given  <div id="my-id"><table> ....data.... </table></div>
const container = document.getElementById('my-id');
const grid = new GridViewScroll({
  table: container.querySelector<HTMLTableElement>(':scope > table')!,
  container: container,
  freezeHeaderRows: 1,
  freezeLeftColumns: 1,
});

grid.enhance();

// prevent resize grip of container from visibly "floating away"
const maxContainerSize = grid.maxContainerSize();
container.style.maxWidth = `${maxContainerSize.width}px`;
container.style.maxHeight = `${maxContainerSize.height}px`;

// (you may want to get a polyfill for ResizeObserver)
new ResizeObserver(() => grid.resize()).observe(container);

Of course, as it is a complete rewrite, it is bound to have no small number of bugs. But it also fixes some bugs from the original.