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