RFR: 8290040: Provide simplified deterministic way to manage listeners [v9]
John Hendrikx
jhendrikx at openjdk.org
Fri Dec 2 08:56:38 UTC 2022
On Fri, 2 Dec 2022 01:06:16 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Adjust Node
>>
>> - Fixed javadoc
>> - Added comment for code that avoid eager instantiation
>> - Changed `isShown` to use property if it is available
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1443:
>
>> 1441: .flatMap(Window::showingProperty); // note: the null case is handled by the delegate with an orElse(false)
>> 1442:
>> 1443: shown = new ReadOnlyBooleanDelegate(Node.this, "shown", ov);
>
> `shownProperty()` might be used quite a lot, have you considered using a specialized implementation that doesn't need several intermediate observable values?
I think that this is already optimized enough by being lazily instantiated (ie. it won't be created if never used). As a second layer, these are fluent bindings which only register listeners when observed.
Creating a specialized implementation should IMHO replicate the lazy behavior (to avoid too many listeners on window/scene when the property isn't actually observed) which is non-trivial and would require an extensive test. This risks creating bugs now (or later) of which there have been quite a few with similar custom properties -- it's easy to accidentally cache an "old" value or forgetting to clear a cached "current" value when invalidated.
In short, I think optimized custom implementations have been and still are a source of bugs, with no evidence that these optimizations are worth while..
-------------
PR: https://git.openjdk.org/jfx/pull/830
More information about the openjfx-dev
mailing list