RFR: 8252935: Add treeShowing listener only when needed
Kevin Rushforth
kcr at openjdk.java.net
Wed Sep 9 21:35:59 UTC 2020
On Tue, 21 Apr 2020 14:22:46 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> @kevinrushforth I've updated this PR now with proper listeners added in again for PopupWindow and
>> ProgressIndicatorSkin. In short, the functionality to support the `treeShowingProperty` has been extracted to a
>> separate class `TreeShowingExpression` which is now used by the two classes mentioned. All tests pass, including the
>> memory leak tests that failed before.
>> The issue JDK-8090322 you mentioned actually cautioned for not adding such listeners for all nodes and seemed to
>> propose the kind of solution I constructed here with a separate class for the `treeShowingProperty`:
>>> 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.
>
> @kevinrushforth
>
> I have another working alternative, but won't make a PR for it until it is more clear which direction the TableView /
> TreeTableView performance fixes are going to take.
> The alternative would convert the `treeShowing` property in `Node` into a "lazy" property (something which JavaFX does
> not support directly at the moment). A lazy property only binds to its dependencies when it is observed itself (so in
> this specific case, only when PopupWindow or ProgressIndicatorSkin are making use of it). This means the property
> stays a part of `NodeHelper` but will only register its listeners on Window and Scene when it is observed itself.
> Such lazy properties could be of great use in JavaFX in general, not just in this case.
@hjohn Per [this message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-September/027534.html) on the
openjfx-dev mailing list, I have filed a new JBS issue for this PR to use. Please change the title to:
8252935: Add treeShowing listener only when needed
-------------
PR: https://git.openjdk.java.net/jfx/pull/185
More information about the openjfx-dev
mailing list