JDK-8252811 : The list of cells in a VirtualFlow is cleared every time the number of items changes
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: openjfx11,openjfx15
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-09-04
  • Updated: 2021-02-02
  • Resolved: 2020-09-19
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx11.0.10Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
When new items are constantly added to a TableView control, the calls to com.sun.javafx.binding.ExpressionHelper.removeListener() (that is part of https://bugs.openjdk.java.net/browse/JDK-8185886 and https://github.com/openjdk/jfx/pull/108) are triggered by the change of the number of cells in VirtualFlow::setCellCount.

This happens every time the number of items in the TableView control 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 (usually 5 to 20 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.

This seems unnecessary, as the number of actual cells/nodes doesn't change that much and the VirtualFlow manages to update them properly, however, it causes very bad performance and high CPU usage.

This issue should explore the option of removing the call to:

sheetChildren.clear();

taking care of a possibly very old comment:

// 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.

If we could get some information about RT-13965, we might see if it is something to be taken into account or not.

Comments
Changeset: d10f948e Author: Jose Pereda <jpereda@openjdk.org> Date: 2020-09-19 19:41:41 +0000 URL: https://git.openjdk.java.net/jfx/commit/d10f948e
19-09-2020

This is one of 4 discrete performance issues split out from JDK-8185886. It is under review here: https://github.com/openjdk/jfx/pull/298
08-09-2020

After adding "RT - 13965" in the description it is clear that the related JBS issue is https://bugs.openjdk.java.net/browse/JDK-8113318. It relates to an Ensamble's sample performance issue and links a test case (KeyEvent.java), from 2011. From the comments: > I was able to see that rendering time was steadily increasing with every pulse, which indicated that a growing number of dirty nodes was likely to be the cause. [...] One suspicious piece of code is in com.sun.javafx.scene.control.skin.VirtualFlow.java. I can see that Group sheet accumulates child nodes but never releases them. I've run the sample printing the dirty nodes size, and with or without calling `sheetChildren.clear(); `, and I don't see any issue with a growing number of dirty nodes. In fact, it keeps steady when pressing down a key (dirtyNodesSize = 19) in both cases, if only the first item is removed when size reaches 8. In case clear is applied when size is 8 instead, it is a little bit worse for current code (alternates dirtyNodesSize = 42 / dirtyNodesSize = 1) vs without `sheetChildren.clear(); ` (alternates dirtyNodesSize = 31 / dirtyNodesSize = 1). In conclusion, I don't see a reason to keep calling `sheetChildren.clear(); `: not only it is not necessary (there is no memory leak without it), but it also causes a performance drop when new items are added constantly to the control.
08-09-2020