On Thu, 2 Oct 2025 19:47:41 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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains two additional
> commits since the last revision:
>
> - Merge branch 'master' of https://github.com/openjdk/jfx into
> 8359599-refresh-recreates-all
> - Calling refresh() for all virtualized controls recreates all cells instead
> of refreshing the cells
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?
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java
line 368:
> 366: JMemoryBuddy.assertCollectable(ref);
> 367: }
> 368:
Is there another test that verifies that cells are garbage collectable? For
example, in the case where a table / list / tree table becomes smaller
visually, I think that it should then perhaps discard some cells that then
should be collectable?
-------------
PR Review: https://git.openjdk.org/jfx/pull/1830#pullrequestreview-3297470172
PR Review Comment: https://git.openjdk.org/jfx/pull/1830#discussion_r2400825922
