On Sun, 5 Oct 2025 17:11:51 GMT, Marius Hanl <[email protected]> wrote:
>>> > I think the changes look good. I'm a bit confused in the performance >>> > table with what is meant with the `50 ms -> 0 ms` in the "after" cases >>> > though? >>> >>> Every `refresh()` will trigger 2 layouts for some reason, where the second >>> one does nothing as nothing is dirty, so basically a noop. I can have a >>> look into that (maybe as a follow up?) but I remember that this happens >>> sometimes in general for the `VirtualFlow` and we should check that >>> generally at one point. >> >> No need to address that in this PR, I was just confused what the numbers >> meant (shouldn't the `before` column than not also have `X ms -> 0 ms`?). >> So it seems like quite a good performance improvement. >> >> As a side note, even 30-40 ms seems incredibly slow, that's bound to create >> noticeable input lag or frame skips :/ How many cells were visible? 1000 >> or 100x1000? If the latter, than 30-40 ms seems okayish. > >> As a side note, even 30-40 ms seems incredibly slow, that's bound to create >> noticeable input lag or frame skips :/ How many cells were visible? 1000 or >> 100x1000? If the latter, than 30-40 ms seems okayish. > > I agree. One problem here is, that all cells will be updated (via > `updateItem`) of a `TableRow`, even if not visible. > > I counted all `updateItem` calls, results below. > > Code from Benchmark above: > - `TableRow` `updateItem`: 78 > - `TableCell` `updateItem`: 7800 > > 39 rows are displayed, and they are updated twice (first with `-1` to reset, > then with the actual index). And all rows have 100 cells. > > A `TableCell` `updateItem` without any code (no `setText`, no `setGraphic`) > is indeed faster, around 10-20ms. > Looking into the code, there are some unnecessary `requestLayout` calls as > well. @Maran23 Can you add the performance test program you ran to validate the performance numbers to this PR? A new directory under `tests/performance` seems a good place for it. I'll review the CSR. @johanvos had some comments earlier, so allow time for him to respond as well. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1830#issuecomment-3393245504
