On Sun, 5 Oct 2025 20:10:02 GMT, Marius Hanl <[email protected]> wrote:
>> When calling `refresh()` on virtualized Controls (`ListView`, `TreeView`,
>> `TableView`, `TreeTableView`), all cells will be recreated completely,
>> instead of just refreshing them.
>>
>> This is because `recreateCells()` of the `VirtualFlow` is called when
>> `refresh()` was called. This is not needed, since refreshing the cells can
>> be done much cheaper with `rebuildCells()`.
>>
>> This will reset all cells (`index = -1`), add them to the pile and fill them
>> back in the viewport with an index again. This ensures `updateItem()` is
>> called.
>>
>> The contract of `refresh()` is also a big vague, stating:
>>
>> Calling {@code refresh()} forces the XXX control to recreate and repopulate
>> the cells
>> necessary to populate the visual bounds of the control.
>> In other words, this forces the XXX to update what it is showing to the
>> user.
>> This is useful in cases where the underlying data source has changed in a
>> way that is not observed by the XXX itself.
>>
>>
>> As written above, recreating is not needed in order to fulfull the contract
>> of updating what is shown to the user in case the underlying data source
>> changed without JavaFX noticing (e.g. calling a normal Setter without any
>> Property and therefore listener involved).
>>
>> ----
>>
>> Benchmarks
>> -
>>
>> Setup:
>> - `TableView`
>> - `100 TableColumns`
>> - `1000 Items`
>>
>> Calling `refresh()` with a `Button` press.
>>
>> | WHAT | BEFORE | AFTER |
>> | - | - | - |
>> | Init | 0ms |0 ms |
>> | Init | 0ms | 0 ms |
>> | Init | 251 ms | 246 ms |
>> | Init | 47 ms | 50 ms |
>> | Init | 24 ms | 5 ms |
>> | Refresh Nr. 1 | 238 ms | 51 ms -> 0ms |
>> | Refresh Nr. 2 | 282 ms | 35 ms -> 0ms |
>> | Refresh Nr. 3 | 176 ms | 36 ms -> 0ms |
>> | Refresh Nr. 4 | 151 ms | 39 ms -> 0ms |
>> | Refresh Nr. 5 | 156 ms | 34 ms -> 0ms |
>>
>>
>>
>> <details>
>> <summary>Benchmark code</summary>
>>
>>
>> import javafx.application.Application;
>> import javafx.beans.property.SimpleStringProperty;
>> import javafx.scene.Scene;
>> import javafx.scene.control.Button;
>> import javafx.scene.control.IndexedCell;
>> import javafx.scene.control.TableColumn;
>> import javafx.scene.control.TableRow;
>> import javafx.scene.control.TableView;
>> import javafx.scene.control.skin.TableViewSkin;
>> import javafx.scene.control.skin.VirtualFlow;
>> import javafx.scene.layout.BorderPane;
>> import javafx.scene.layout.HBox;
>> import javafx.stage.Stage;
>>
>> public class FXBug {
>>
>> public static void main(String[] args) {
>> Application.launch(FxApp.class, args);
>> }
>>
>> ...
>
> Marius Hanl has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - remove newline from imports
> - assert row creation count as well
I left one comment on the docs. I'll leave it to the other reviewers to test
and review the implementation changes.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1830#pullrequestreview-3327030399
