The "dynamic update performance" performance issue we are seeing is a
regression from JDK-8090322. In this change the Node treeShowing
property was introduced. The JDK-8090322 warns in its description about:
""" Considerations:
* This is too expensive to calculate for all nodes by default. So the
simplest way to provide it would be a special binding implementation or
a util class. Where you create a instance and pass in the node you are
interested in. It can then register listeners all the way up the tree
and listen to what it needs. """
The above comment is warning against the fact that registering listeners
for EACH Node on Window and Scene is going to be a performance issue.
As nodes can number in the 1000's or 10.000's, that's a lot of listeners
to store in a List data structure.
PR 185 targets this issue and implements the feature in JDK-8090322 in
the way that was suggested in the above comment, instead of how it
currently is implemented (registering listeners for every Node, just in
case someone needs the treeShowing property).
It's scope is not as broad as it would seem (because of a change in
Node). It effectively only makes a small change in two controls
(PopupWindow and ProgressIndicatorSkin).
--John
On 31/08/2020 16:54, Jeanette Winzenburg wrote:
Looking at the examples provided in 108/125: apart from both having many
columns (> 300 makes them really nasty) they differ in
Table content:
125 - static data
108 - items are frequently modified (added)
Perceived performance:
125 - vertical scrolling: thumb/content lags behind mouse
108 - with enough columns, all interaction is sluggish (mouse, keys,
..), scrolling being just one of them
Both have examples, running those against the suggested fixes (with 108a
for Jose's approach)
125 - fixes its own example, does nothing for the other
108, 108a, 185 - improves its own example, does nothing for other
So we seem to have multiple issues that are (mostly) orthogonal: one
being the broken/missing horizontal virtualization (125), the other
related to dynamic update of table content (108, 108a, 185). We need to
solve both, the solution/s for one looks (mostly?) unrelated to the
solution to the other.
125 is the only one PR for its use-case, and it seems to do its job.
From those targeting the dynamic data update all except Jose's (not yet
formalized into a PR) approach feel too broad: table's reaction to items
modifications is .. suboptimal in more than one aspect. Changing overall
notification implementation to improve that, sounds like covering it up.
-- Jeanette
Zitat von Kevin Rushforth <kevin.rushfo...@oracle.com>:
Sorry for the badly formatted html. Here it is again.
I see some progress being made on the {Tree}TableView performance
issue. To summarize where I think we are:
There are currently 2 different approaches under review:
1. https://github.com/openjdk/jfx/pull/108 : optimization in
javafx.base to make removing listeners faster
2. https://github.com/openjdk/jfx/pull/125 : optimization in TableView
to reduce the number of add / removes
In addition, the following is a WIP PR that the author thinks could be
a 3rd approach:
3. https://github.com/openjdk/jfx/pull/185 : optimization in scene
graph to avoid install treeShowing listeners on Window and Scene for
all nodes
Jose has proposed a 4th approach as a comment to PR #108, and as I
understand it, he will propose a PR shortly.
4. Don't clear the list of children in a VirtualFlow when the number
of items changes.
So the first thing that is needed is to evaluate the approaches and
decide which one to pursue.
Options 1 and 3 are more broad in their scope, option #2 is more
targeted (to TableView), but within that area is a larger change.
Option #3 would remove the (internal) treeShowing property as a
generally available concept and only use it for controls like
ProgressIndicator that really need it. Option #4 affects only controls
that use VirtualFlow (ListView, TableVIew, TreeTableView), and seems
not to be a large change (presuming we can verify that no leak is
introduced).
I note that these fixes are not mutually exclusive, but I do think we
need to settle on a primary approach and use that to fix this issue.
If there are still performance problems after that fix, we can
consider one (or more) of the others.
Comments?
-- Kevin
