perf: virtualize the notification list - #3319

I've faced this freezing issue this week too; it might have been a recently introduced bug.
But this is awesome! I've been wanting to test out virtualized lists for a while. HOWEVER, tanstack's implementation is not the most performant one. I'd argue that LegendList does the job much better: https://www.legendapp.com/open-source/list/v3/react/examples/virtual-list-comparison/

In fact, the freezing was probably what you fixed in 6320175 (#3316)?

In fact, the freezing was probably what you fixed in 6320175 (#3316)?

Shall we pull this out to a separate PR

pulled the cache write fix out to #3320, that one is the mark-as-read freeze.

for trying out LegendList, benched both on the same 1274-notification list:

┌───────────────────────┬───────────────┬─────────────┐
│                       │ react-virtual │ LegendList  │
├───────────────────────┼───────────────┼─────────────┤
│ frame p50             │ 16.7ms        │ 16.7ms      │
├───────────────────────┼───────────────┼─────────────┤
│ frame p95             │ 33.4–66.6ms   │ 33.2–33.4ms │
├───────────────────────┼───────────────┼─────────────┤
│ frames >50ms per pass │ 0–14          │ 0           │
├───────────────────────┼───────────────┼─────────────┤
│ rows mounted at rest  │ 11            │ 15          │
└───────────────────────┴───────────────┴─────────────┘

happy to swap, one catch: legend measures nothing in happy-dom, no layout and its ResizeObserver never fires, so the list renders zero rows there. the windowing tests would have to move to a real browser. ok with that?

Thanks @fmguerreiro.

I'd be leaning to keeping within the tanstack family for "simplicity". It's a positive improvement from current state

@setchy there is no reason for us to stick with tanstack in this case just for the sake of it. It is an isolated package with zero relation to the rest. I think the numbers speak for themselves in terms of frame consistency.

@setchy there is no reason for us to stick with tanstack in this case just for the sake of it. It is an isolated package with zero relation to the rest. I think the numbers speak for themselves in terms of frame consistency.

i'll let you have the final say on whichever path you'd like to go down. you know my thoughts ;)

Alright, so I've done some local testing and got similar results to @fmguerreiro. I ran these on an M5 Macbook, so I throttled the CPU a bit, but older machines should definitely face this.

So, on to the results. I tested both with 2,000 notifications in production Chrome, ran it 3 times and these are the averages:

The main reason I’d choose LegendList here is frame consistency under load, and I think that's mostly what we are targeting. I see no reason to make a performance improvement, without making sure that we've squeezed it all, and LegendList is known to be a legend of a package. As someone who has worked with React Native as well, it is a godsend.

In any case, the implementation is also simpler since LegendList handles row positioning, measurement and scroll-container sizing. That removes code we would otherwise need to maintain ourselves.

Sounds good to me :)