RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v3]

[Kevin Rushforth]

On Tue, 11 Apr 2023 07:28:36 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>>> Well, I am just following the specification here, but JavaFX itself isn't...
>>> 
>>> We may need to make a decision here. JavaFX is not consistent itself where it applies `equals` and where it uses reference equality:
>> 
>> Alright, so this is fine for this PR.
>> 
>> As for the equality test, we will need to revise the implementation note (that I wrote) to explain correctly what is used where and consider the changes you proposed. ReactFX uses `equals` internally, and it was very hard to debug why changing to its observables caused a change in the behavior, which is why I'm extra careful about this issue.
>> My idea was to allow the user to specify their own equality checks when registering a listener, but that requires a lot more thought.
>
>> > Well, I am just following the specification here, but JavaFX itself isn't...
>> > We may need to make a decision here. JavaFX is not consistent itself where it applies `equals` and where it uses reference equality:
>> 
>> Alright, so this is fine for this PR.
>> 
>> As for the equality test, we will need to revise the implementation note (that I wrote) to explain correctly what is used where and consider the changes you proposed. ReactFX uses `equals` internally, and it was very hard to debug why changing to its observables caused a change in the behavior, which is why I'm extra careful about this issue. My idea was to allow the user to specify their own equality checks when registering a listener, but that requires a lot more thought.
> 
> Well at least you only described what JavaFX has been doing all along then.
> 
> I am convinced that reference equality is the right way to go; ~unwanted~ equal changes can be ignored in the change listener (or something that wraps your change listener).  An ~unwanted~ equal change would only occur when the type involved is mutable, which you would have to ignore at your own peril -- once you have the wrong instance, you will either see changes to the "old" instance that you don't want, or not pick up on any direct changes to the new instance that you decided to ignore.
> 
> Luckily, most stuff JavaFX is doing is using immutable types.  It gets in trouble a bit with all the Set/List/Map wrappers, which is what the test pointed out (you need to use `InvalidationListener`s there, as they still fire on reference equality changes).

@hjohn Can you merge in the latest master? Your branch is ~ 2 months out of date and that will ensure we get an updated test run.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1056#issuecomment-1512946926