RFR: 8359599: Calling refresh() for all virtualized controls recreates all cells instead of refreshing the cells [v2]

[Kevin Rushforth]

On Sun, 5 Oct 2025 17:11:51 GMT, Marius Hanl <mhanl at openjdk.org> 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