RFR: 8351276: Prevent redundant computeValue calls when a chain of mappings becomes observed
John Hendrikx
jhendrikx at openjdk.org
Fri Mar 7 22:57:59 UTC 2025
On Fri, 7 Mar 2025 20:38:38 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
>> 8351276: Prevent redundant computeValue calls when a chain of mappings becomes observed
>
> 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 ___."
If you add the listener first, then `ExpressionHelper` will do a `getValue` causing the 4 mappings to be computed but without being able to cache them as the bindings are not yet aware there is a listener present now (and so will remain "invalid"). The observable belonging to the last mapping will then have its `observeSources` called. This adds another listener (on the 3rd mapping, via the invalidation listener endpoint, which had the same ordering issue). Again, `ExpressionHelper` will try get the value, and 3 mappings will be recomputed (the 4th mapping is not computed again as its `observeSources` was called already). This continues when the listener is added on the 2nd mapping (and 2 mappings are recomputed), and on the first mapping (computing that one, one final time).
So I guess the continuation could be something like:
"... because the process of adding a listener triggers the computation of the observable's value which, when called before #observeSource was called, won't be cached yet."
> 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());
> }
Not really, I'd prefer to be explicit in tests.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985815678
PR Review Comment: https://git.openjdk.org/jfx/pull/1730#discussion_r1985816609
More information about the openjfx-dev
mailing list