RFR: 8351276: Prevent redundant computeValue calls when a chain of mappings becomes observed

[Nir Lisker]

On Thu, 6 Mar 2025 15:22:58 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> 8351276: Prevent redundant computeValue calls when a chain of mappings becomes observed

modules/javafx.base/src/main/java/com/sun/javafx/binding/LazyObjectBinding.java line 62:

> 60:         updateSubscriptionBeforeAdd();
> 61: 
> 62:         super.addListener(listener);

I noticed that reverting this change causes all the `GivenAQuadMappedObservable` to fails, but reverting the add `ChangeListener` method change causes only change listener tests to fail. Is this correct? Shouldn't there be a tests that fails just the invalidation listener code?

modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line 378:

> 376:          * can first become observed (allowing caching) and are then queried (likely
> 377:          * getting the value from the cache). Doing this the other way around would
> 378:          * result in the value (or chain of values) being computed twice.

If I understand correctly, it will be computed twice because the observable would not be able to cache the value yet. It will compute it once without caching with `getValue()`, and then again with `addListener(...)`. If this is correct, I suggest writing a short continuation: 
"Doing this the other way around would result in the value (or chain of values) being computed twice because ___."

modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java line 61:

> 59:     private final InvalidationListener invalidationListener = obs -> invalidations++;
> 60: 
> 61:     private StringProperty property = new SimpleStringProperty("Initial");

`final`?

modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java line 65:

> 63:     @Nested
> 64:     class GivenAQuadMappedObservable {
> 65:         AtomicInteger calls1 = new AtomicInteger(0);

Empty line after the class declaration.

modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java line 80:

> 78:             assertEquals(0, calls2.get());
> 79:             assertEquals(0, calls3.get());
> 80:             assertEquals(0, calls4.get());

Since these assertions are a repeating pattern, do you want to extract it to a reusable method?


private void assertValue(int val) {
    assertEquals(val, calls1.get());
    assertEquals(val, calls2.get());
    assertEquals(val, calls3.get());
    assertEquals(val, calls4.get());
}

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985752123
PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985677539
PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985679166
PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985680283
PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985667361