petyosi / react-virtuoso

The most powerful virtual list component for React
https://virtuoso.dev
MIT License
5.25k stars 301 forks source link

useScrollTop is very slow #204

Closed FezVrasta closed 2 years ago

FezVrasta commented 3 years ago

Hi, I'm testing @next and I noticed most of the time on scroll is spent on the useScrollTop hook, it leads to very long frames.

image

image

petyosi commented 3 years ago

Can you publish the test case somewhere?

FezVrasta commented 3 years ago

Unfortunately it's not an open source app so I can't 🙁

FezVrasta commented 3 years ago

I wonder if you could use IntersectionObserver as a more performance effective replacement to scrollTop? From what I see you are using the hook to accomplish a similar result?

petyosi commented 3 years ago

@FezVrasta I am not sure that I will be able to get away with intersection observer - nor if the slowdown you observe is not due to the underlying callbacks and updates. I will investigate - thank you very much for the suggestion and testing.

FezVrasta commented 3 years ago

I'm quite convinced the issue resides in Virtuoso, I recently tried to swap it with react-window and the performances were way better, but of course I lost all the nice features that Virtuoso provides out of the box.

FezVrasta commented 3 years ago

Something interesting to observe is that there are 3 calls to scrollTop on the same action:

image

In the screenshot above, the 3 long tasks are all scroll events. I would expect a single scroll event per call?

petyosi commented 3 years ago

That actually gives me a clue. Without any code reveal, can you post what props do you pass to the component?

FezVrasta commented 3 years ago

This is the relevant code (I use react-table):

const footer = useCallback(() => {
  if (firstRow == null) {
    return null;
  }
  prepareRow(firstRow);

  return Array.from({
    length: Math.min(10, totalCount - rows.length),
  }).map((_, i) => <PlaceholderRow row={firstRow} key={i} />);
}, [prepareRow, rows.length, firstRow, totalCount]);

const components = useMemo(
  () => ({
    EmptyPlaceholder: loading ? null : EmptyComponent,
  }),
  [loading]
);

const itemContent = useCallback(
  (index) => {
    const row = rows[index];
    prepareRow(row);
    return (
      <Row
        row={row}
        onRowClick={onRowClick}
        onAddToQueue={onAddToQueue}
        onStartRead={onStartRead}
        onView={onView}
        onRemove={onRemove}
        onCompare={onCompare}
        tab={tab}
        isScrolling={isScrolling}
      />
    );
  },
  [
    isScrolling,
    onAddToQueue,
    onCompare,
    onRemove,
    onRowClick,
    onStartRead,
    onView,
    prepareRow,
    rows,
    tab,
  ]
);

<Virtuoso
  css={`
    flex: 1;
    overflow-x: hidden;
    ${style};
  `}
  totalCount={rows.length}
  endReached={onFetchMore}
  isScrolling={setIsScrolling}
  overscan={1000}
  footer={footer}
  components={components}
  defaultItemHeight={51}
  itemContent={itemContent}
/>
petyosi commented 3 years ago

@FezVrasta thanks to your report, I found some unnecessary re-renders and scrollTop calls. Just published beta.3 - please let me know if you see improvements in your case.

FezVrasta commented 3 years ago

beta.3 seems to be better, but I still see one extra call (beta.2 had 3, so you are going the right way!)

image

I also see this other long frame here: image

image

petyosi commented 3 years ago

@FezVrasta can you try removing the overscan setting? This is how the timeline looks on my side without it: image and with 2000 set. image

FezVrasta commented 3 years ago

I see less long frames but the rows render very slowly (this also happens with overscan but not that much).

Kapture 2020-11-26 at 17 33 37

FezVrasta commented 3 years ago

Is there any progress on this? I can reproduce the issue on 1.5.13

image the above is with no overscan set

petyosi commented 3 years ago

I haven't discovered any additional general performance improvements. Again, the best way to help me here is with a way for me to reproduce the slowness you observe on my side.