RFR: 8293836: Rendering performance degradation at bottom of TableView with many rows [v2]

[Marius Hanl]

On Tue, 30 May 2023 10:39:28 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> Only update the VirtualFlow parameters in case the size of a cell has changed.
>> 
>> The fixes for JDK-8298728 and JDK-8277785 introduced an unconditional recalculation in case the size of a cell is set. This recalculation is only needed in case the size of that specific cell has changed.
>> Fix for JDK-8293836
>
> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove newline

Have one question. Otherwise looks good to me. Tested with a lot of data + with and without fixed cell size.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java line 3083:

> 3081:         if (itemSizeCache.size() > cellIndex) {
> 3082:             Double oldSize = itemSizeCache.get(cellIndex);
> 3083:             double newSize = isVertical() ? cell.getLayoutBounds().getHeight() : cell.getLayoutBounds().getWidth();

If we have a fixed cell size set, can we save the call to get the layout bounds here?
Since this is also done above in `getOrCreateCellSize` (`getFixedCellSize()`).
And the layout bounds are calculated lazily, so this may improve the performance here as well.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java line 3085:

> 3083:             double newSize = isVertical() ? cell.getLayoutBounds().getHeight() : cell.getLayoutBounds().getWidth();
> 3084:             itemSizeCache.set(cellIndex, newSize);
> 3085:             if ((oldSize != null) && !oldSize.equals(newSize)) {

Minor: Braces are not needed here

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java line 3088:

> 3086:                 int currentIndex = computeCurrentIndex();
> 3087:                 double oldOffset = computeViewportOffset(getPosition());
> 3088:                 if ((cellIndex == currentIndex) && (oldOffset != 0)) {

Minor: Braces are not needed here

-------------

PR Review: https://git.openjdk.org/jfx/pull/1098#pullrequestreview-1456254974
PR Review Comment: https://git.openjdk.org/jfx/pull/1098#discussion_r1213648251
PR Review Comment: https://git.openjdk.org/jfx/pull/1098#discussion_r1213645083
PR Review Comment: https://git.openjdk.org/jfx/pull/1098#discussion_r1213645163