cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
40 stars 25 forks source link

Evaluate virtualized/windowed React #225

Open allisonkarlitskaya opened 7 months ago

allisonkarlitskaya commented 7 months ago

After the changes of #210, we are easily able to enter directories with a very large number of files, with very fast response from the backend code.

Adding ~5000-10000 cards to the DOM isn't so great, though...

There are some tools available to us here in vanilla react:

and even a Patternfly extension:

We need to evaluate the use of one of these, or figure out a different solution.

garrett commented 7 months ago

This is likely one performance bottleneck with a solution:

I'm guessing we're still embedding SVGs. We shouldn't with that many files, as those all become individual documents in the DOM. We should treat them like images and/or use CSS to render them instead.

At least from main:

image

Additionally, our complexity is absurd. Here's the layout with the container for the directory+files highlighted plus the first item on the list:

image

Everything under the blue line is one item. And there are nested grids in flexes in flexes in a grid! It has to compute the size of all the parts of elements, the elements themselves, and the grid layout and the page layout all at the same time, interdependently!

...And events bound all over the place, instead of binding one at the top level and using event bubbling! :exploding_head:

(In the computed DOM in the screenshot, you can see flex and grid layouts with bubbles, and you can also see event bubbles.)

If we fix these problems and it'll perform much, much better... without any absurd tricks.

Overall summary:

  1. Refer to images for re-use (either using <img> or CSS), not use thousands of embedded SVG documents, which does not scale well.
  2. Reduce DOM complexity. It doesn't need to be nested this deep to get a layout we want. I don't know if this means we need to make a custom component or use PF components in a special way.
  3. Reduce layout complexity. (Goes hand in hand with reducing DOM complexity, but not completely tied to it.) The browser will struggle a bit with having thousands of different mix-and-match layouts nested like this, as all of them have to be interdependently computed to solve for the layout of the page. There are some CSS-based optimizations to make this less of a problem, but reducing the number of items needing layout would help the most.
  4. Don't bind so many event handlers to elements in the tree, but use event bubbling and bind at the parent instead. (You attach one event listener at a common parent higher up the DOM tree and listen for a matching event and dispatch some action for the child object when it matches.) Each event handler bogs down the DOM and having thousands does not scale well.

We should do all of these. Every one of these on their own would help with performance.

jelly commented 7 months ago

Regarding event handlers, every item has three event handlers:

              onClick={ev => handleClick(ev, file)}
              onContextMenu={(e) => {
                  e.stopPropagation();
                  setSelectedContext(file);
                  if (selected.length === 1 || !selected.includes(file))
                      setSelected([file]);
              }}
              onDoubleClick={() => onDoubleClickNavigate(file)}
jelly commented 7 months ago

I've tried out react-window and I've come to the conclusion that we need to make the "card" a fixed size for this to work so far I noticed that they are mostly the same size, except the height varies depending on the truncation of the directory/filename.

We probably want to use [https://react-window.vercel.app/#/examples/grid/variable-size](a variablegrid) for the card overview.

For the size of the Grid we probably want to use react-virtualized-auto-sizer so we don't have to calculate how much space the grid needs to take up.

A big issue is that we use our own ListingTable here which we will need to modify to support windowed. Maybe it's an idea to start a new Cockpit playground for the ListingTable and try to get a virtualized table implemented there with a simple <Card> and