RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

yosbits github.com+7517141+yososs at openjdk.java.net
Wed Aug 26 17:46:38 UTC 2020

On Wed, 26 Aug 2020 15:40:57 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> I have attached a code sample. If you use OpenJFX 16-ea+1 and run visual VM and look at the hotspots in the JavaFX
>> thread, you can see that about 45% of the time in the JavaFX thread is spent in removeListener calls.
>> Note: In CPU settings of VisualVM, I removed all packages from the "Do not profile packages section".
>> [JavaFXSluggish.java.zip](https://github.com/openjdk/jfx/files/5130298/JavaFXSluggish.java.zip)
> So far, there are two alternative fixes for the bad performance issue while scrolling TableView/TreeTableViews:
> - this one #108, that tries to improve the performance of excessive number of `removeListener` calls
> - the WIP #185 that avoids registering two listeners (on Scene and on Window) for each and every Node.
> For the case presented, where new items are constantly added to the TableView, the trace of calls that reach
> `com.sun.javafx.binding.ExpressionHelper.removeListener()` is something like this:
> ![TraceOpenJFX16ea1](https://user-images.githubusercontent.com/2043230/91322208-c2ba9900-e7bf-11ea-8918-cdbecd457cf2.png)
> As can be seen, all those calls are triggered by the change of the number of cells in
> [VirtualFlow::setCellCount](https://github.com/openjdk/jfx/blob/master/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java#L804).
> Whenever the cell count changes there is this
> [call](https://github.com/openjdk/jfx/blob/master/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java#L843):
> sheetChildren.clear();
> This happens every time the number of items in the `TableView` changes, as the `VirtualFlow` keeps track of the number
> of virtual cells (`cellCount` is the total number of items) while the number of actual cells or number of visible nodes
> used is given by `sheetChildren.size()`.  This means that all the actual cells (nodes) that are used by the
> `VirtualFlow` are disposed and recreated all over again every time the number of items changes, triggering all those
> calls to unregister those nodes from the scene that ultimately lead to remove the listeners with
> `ExpressionHelper.removeListener`.  However, this seems unnecessary, as the number of actual cells/nodes doesn't change
> that much, and causes this very bad performance.  On a quick test over the sample posted, just removing that line gives
> a noticeable improvement in performance..
> There is a concern though due to the
> [comment](https://github.com/openjdk/jfx/blob/master/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java#L839):
> // Fix for RT-13965: Without this line of code, the number of items in
> // the sheet would constantly grow, leaking memory for the life of the
> // application. This was especially apparent when the total number of
> // cells changes - regardless of whether it became bigger or smaller.
> sheetChildren.clear();
> There are some methods in VirtualFlow that already manage the lifecycle of this list of nodes (clear, remove cells, add
> cells, ...), so I don't think that is the case anymore for that comment: the number of items in the sheet doesn't
> constantly grow and there is no memory leak.  Running the attached sample for a long time, and profiling with VisualVM,
> shows improved performance (considerable drop in CPU usage), and no issues regarding memory usage.
> So I'd like to propose this third alternative, which would affect only VirtualFlow and the controls that use it, but
> wouldn't have any impact in the rest of controls as the other two options (as `ExpressionHelper` or `Node` listeners
> wouldn't be modified).  Thoughts and feedback are welcome.

I confirmed the sample code (JavaFX Sluggish),
This is not scroll performance
It seems to reproduce the additional performance issue.
Therefore, it is not considered appropriate as a fix for JDK-8185886.
I know you are reproducing another performance issue, but
I'm proposing to fix scrolling performance issues in #125.


PR: https://git.openjdk.java.net/jfx/pull/108

More information about the openjfx-dev mailing list