Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]
On Mon, 7 Nov 2022 20:29:16 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix javadoc error > > I am not a native speaker, but to me `whenever` sounds event better than > `conditionOn`. I might be wrong, but it does suggest that the binding > becomes active *again* when the condition turns `true`. @andy-goryachev-oracle @mstr2 would one of you be willing to review this one as well? It should be close to its final state now. - PR: https://git.openjdk.org/jfx/pull/830
Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]
On Wed, 26 Oct 2022 07:56:52 GMT, John Hendrikx wrote:
>> This PR adds a new (lazy*) property on `Node` which provides a boolean which
>> indicates whether or not the `Node` is currently part of a `Scene`, which in
>> turn is part of a currently showing `Window`.
>>
>> It also adds a new fluent binding method on `ObservableValue` dubbed `when`
>> (open for discussion, originally I had `conditionOn` here).
>>
>> Both of these together means it becomes much easier to break strong
>> references that prevent garbage collection between a long lived property and
>> one that should be shorter lived. A good example is when a `Label` is bound
>> to a long lived property:
>>
>>
>> label.textProperty().bind(longLivedProperty.when(label::isShowingProperty));
>>
>> The above basically ties the life cycle of the label to the long lived
>> property **only** when the label is currently showing. When it is not
>> showing, the label can be eligible for GC as the listener on
>> `longLivedProperty` is removed when the condition provided by
>> `label::isShowingProperty` is `false`. A big advantage is that these
>> listeners stop observing the long lived property **immediately** when the
>> label is no longer showing, in contrast to weak bindings which may keep
>> observing the long lived property (and updating the label, and triggering
>> its listeners in turn) until the next GC comes along.
>>
>> The issue in JBS also describes making the `Subscription` API public, but I
>> think that might best be a separate PR.
>>
>> Note that this PR contains a bugfix in `ObjectBinding` for which there is
>> another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because
>> the tests for the newly added method would fail otherwise; once it has been
>> integrated to jfx19 and then to master, I can take the fix out.
>>
>> (*) Lazy means here that the property won't be creating any listeners unless
>> observed itself, to avoid problems creating too many listeners on
>> Scene/Window.
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix javadoc error
Looks good. Left a couple of minor comments.
modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1410:
> 1408: /**
> 1409: * Indicates whether or not this {@code Node} is shown. A node is
> considered shown if it's
> 1410: * part of a {@code Scene} which is part of a {@code Window} whose
which -> that
modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1420:
> 1418: * when it is no longer shown.
> 1419: *
> 1420: * @see ObservableValue#when(ObservableValue)
You don't need the `@see` since you linked it in the docs.
-
PR: https://git.openjdk.org/jfx/pull/830
Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]
On Wed, 26 Oct 2022 07:56:52 GMT, John Hendrikx wrote: >> This PR adds a new (lazy*) property on `Node` which provides a boolean which >> indicates whether or not the `Node` is currently part of a `Scene`, which in >> turn is part of a currently showing `Window`. >> >> It also adds a new fluent binding method on `ObservableValue` dubbed `when` >> (open for discussion, originally I had `conditionOn` here). >> >> Both of these together means it becomes much easier to break strong >> references that prevent garbage collection between a long lived property and >> one that should be shorter lived. A good example is when a `Label` is bound >> to a long lived property: >> >> >> label.textProperty().bind(longLivedProperty.when(label::isShowingProperty)); >> >> The above basically ties the life cycle of the label to the long lived >> property **only** when the label is currently showing. When it is not >> showing, the label can be eligible for GC as the listener on >> `longLivedProperty` is removed when the condition provided by >> `label::isShowingProperty` is `false`. A big advantage is that these >> listeners stop observing the long lived property **immediately** when the >> label is no longer showing, in contrast to weak bindings which may keep >> observing the long lived property (and updating the label, and triggering >> its listeners in turn) until the next GC comes along. >> >> The issue in JBS also describes making the `Subscription` API public, but I >> think that might best be a separate PR. >> >> Note that this PR contains a bugfix in `ObjectBinding` for which there is >> another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because >> the tests for the newly added method would fail otherwise; once it has been >> integrated to jfx19 and then to master, I can take the fix out. >> >> (*) Lazy means here that the property won't be creating any listeners unless >> observed itself, to avoid problems creating too many listeners on >> Scene/Window. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix javadoc error As per the discussion on the mailing list (https://mail.openjdk.org/pipermail/openjfx-dev/2022-November/036707.html) we've decided to go for `when` as the name for the new method in `ObservableValue`. I've created a CSR now: https://bugs.openjdk.org/browse/JDK-8297719 - PR: https://git.openjdk.org/jfx/pull/830
Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]
On Wed, 26 Oct 2022 07:56:52 GMT, John Hendrikx wrote: >> This PR adds a new (lazy*) property on `Node` which provides a boolean which >> indicates whether or not the `Node` is currently part of a `Scene`, which in >> turn is part of a currently showing `Window`. >> >> It also adds a new fluent binding method on `ObservableValue` dubbed `when` >> (open for discussion, originally I had `conditionOn` here). >> >> Both of these together means it becomes much easier to break strong >> references that prevent garbage collection between a long lived property and >> one that should be shorter lived. A good example is when a `Label` is bound >> to a long lived property: >> >> >> label.textProperty().bind(longLivedProperty.when(label::isShowingProperty)); >> >> The above basically ties the life cycle of the label to the long lived >> property **only** when the label is currently showing. When it is not >> showing, the label can be eligible for GC as the listener on >> `longLivedProperty` is removed when the condition provided by >> `label::isShowingProperty` is `false`. A big advantage is that these >> listeners stop observing the long lived property **immediately** when the >> label is no longer showing, in contrast to weak bindings which may keep >> observing the long lived property (and updating the label, and triggering >> its listeners in turn) until the next GC comes along. >> >> The issue in JBS also describes making the `Subscription` API public, but I >> think that might best be a separate PR. >> >> Note that this PR contains a bugfix in `ObjectBinding` for which there is >> another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because >> the tests for the newly added method would fail otherwise; once it has been >> integrated to jfx19 and then to master, I can take the fix out. >> >> (*) Lazy means here that the property won't be creating any listeners unless >> observed itself, to avoid problems creating too many listeners on >> Scene/Window. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix javadoc error I am not a native speaker, but to me `whenever` sounds event better than `conditionOn`. I might be wrong, but it does suggest that the binding becomes active *again* when the condition turns `true`. - PR: https://git.openjdk.org/jfx/pull/830
Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]
On Wed, 26 Oct 2022 07:56:52 GMT, John Hendrikx wrote: >> This PR adds a new (lazy*) property on `Node` which provides a boolean which >> indicates whether or not the `Node` is currently part of a `Scene`, which in >> turn is part of a currently showing `Window`. >> >> It also adds a new fluent binding method on `ObservableValue` dubbed `when` >> (open for discussion, originally I had `conditionOn` here). >> >> Both of these together means it becomes much easier to break strong >> references that prevent garbage collection between a long lived property and >> one that should be shorter lived. A good example is when a `Label` is bound >> to a long lived property: >> >> >> label.textProperty().bind(longLivedProperty.when(label::isShowingProperty)); >> >> The above basically ties the life cycle of the label to the long lived >> property **only** when the label is currently showing. When it is not >> showing, the label can be eligible for GC as the listener on >> `longLivedProperty` is removed when the condition provided by >> `label::isShowingProperty` is `false`. A big advantage is that these >> listeners stop observing the long lived property **immediately** when the >> label is no longer showing, in contrast to weak bindings which may keep >> observing the long lived property (and updating the label, and triggering >> its listeners in turn) until the next GC comes along. >> >> The issue in JBS also describes making the `Subscription` API public, but I >> think that might best be a separate PR. >> >> Note that this PR contains a bugfix in `ObjectBinding` for which there is >> another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because >> the tests for the newly added method would fail otherwise; once it has been >> integrated to jfx19 and then to master, I can take the fix out. >> >> (*) Lazy means here that the property won't be creating any listeners unless >> observed itself, to avoid problems creating too many listeners on >> Scene/Window. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix javadoc error I think the most important open point for this PR is the name for the new operation to be added to `ObservableValue`. Currently in this PR it is called `when`. Some background first to help with the decision; all fluent methods follow this basic pattern: source.operation( ... ) -> new target observable The proposed operation has a `ObservabeValue` parameter as its condition. It allows you to create a new observable that has the same value as its source, unless the associated condition is `false`, in which case it contains the last seen value of the source (it does **not** change to `null` or empty, so an `orElse` cannot be used to change the value when the condition is `false`.). The proposed operation differs from the other three available fluent methods (`map`, `flatMap` and `orElse`) that in order to determine its result it does not need to continuously observe its source -- the source value has no bearing on whether the target will or will not reflect the source value, only the condition controls this. This is unlike `map` for example, which **must** evaluate the source value to determine the target value, and also unlike a `filter` method (as seen in `Optional` or `Stream`) which must evaluate the source value to see if it passes the filter predicate in order to determine whether the target uses the source value or becomes empty. Because the source value is not needed to reach a decision for this new proposed operation, there is no need to constantly observe the source value for changes. When the conditional is `false`, the observer can be removed until such time that the conditional becomes `true`. This is in contrast to `map`, `orElse` or a potential future `filter` operation which have no choice but to observe their source at all times in order to correctly calculate their target value. There is also a clear intention that the condition changes at some point in the future and that it need not be a one time change (when condition becomes `true` again, the target again starts tracking the source value); if the condition never changes there is no need to use this new operation as one can just use the source observable (if condition is always `true`) or it becomes a constant value (if condition is always `false`). A quick overview of the differences between the existing and new operation: |Time|x|condition|x.map(x -> x * 2)|x.filter(x -> x % 2 == 0)|x.operation(condition)| |:-:|:-:|:-:|:-:|:-:|:-:| |0|2|true|4|2|2| |1|2|false|4|2|2| |2|3|false|6|null (empty)|2| |3|3|true|6|null (empty)|3| |4|4|true|8|4|4| ## Options for the name of this new operation The full signature for the newly proposed operation is: ObservableValue __operation__(ObservableValue condition) Possible options: 1. `while`, `if` 2. `conditionOn`
Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]
> This PR adds a new (lazy*) property on `Node` which provides a boolean which > indicates whether or not the `Node` is currently part of a `Scene`, which in > turn is part of a currently showing `Window`. > > It also adds a new fluent binding method on `ObservableValue` dubbed `when` > (open for discussion, originally I had `conditionOn` here). > > Both of these together means it becomes much easier to break strong > references that prevent garbage collection between a long lived property and > one that should be shorter lived. A good example is when a `Label` is bound > to a long lived property: > > > label.textProperty().bind(longLivedProperty.when(label::isShowingProperty)); > > The above basically ties the life cycle of the label to the long lived > property **only** when the label is currently showing. When it is not > showing, the label can be eligible for GC as the listener on > `longLivedProperty` is removed when the condition provided by > `label::isShowingProperty` is `false`. A big advantage is that these > listeners stop observing the long lived property **immediately** when the > label is no longer showing, in contrast to weak bindings which may keep > observing the long lived property (and updating the label, and triggering its > listeners in turn) until the next GC comes along. > > The issue in JBS also describes making the `Subscription` API public, but I > think that might best be a separate PR. > > Note that this PR contains a bugfix in `ObjectBinding` for which there is > another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because > the tests for the newly added method would fail otherwise; once it has been > integrated to jfx19 and then to master, I can take the fix out. > > (*) Lazy means here that the property won't be creating any listeners unless > observed itself, to avoid problems creating too many listeners on > Scene/Window. John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Fix javadoc error - Changes: - all: https://git.openjdk.org/jfx/pull/830/files - new: https://git.openjdk.org/jfx/pull/830/files/ea546998..46f206aa Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=830=07 - incr: https://webrevs.openjdk.org/?repo=jfx=830=06-07 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jfx/pull/830.diff Fetch: git fetch https://git.openjdk.org/jfx pull/830/head:pull/830 PR: https://git.openjdk.org/jfx/pull/830
