On Mon, 17 Apr 2023 11:25:53 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Description copied from issue: >> >> There are up to two additional invalidations performed that really should be >> avoided, causing downstream fluent bindings to be recomputed with the same >> values. This is very confusing as these should only be called when there is >> an actual change, and not called for the same value multiple times in a row. >> >> These two extra invalidations have two different causes, each causing an >> additional invalidation to be triggered: >> >> 1) ObjectBinding's `isObserved` is delayed slightly. When you add a >> listener, the listener is added internally and the binding is made valid; >> this triggers some downstream activity which checks the `isObserved` status >> to decide whether to start observation of properties -- unfortunately this >> still returns `false` at that time. A work-around for this existed by >> calling `getValue` again in `LazyObjectBinding` with a huge comment >> explaining why this is needed. Although this works, it still means that a >> downstream function like `map` is called an additional time while it should >> really only be called once. >> >> The solution is to ensure `isObserved` returns `true` before the >> `ExpressionHelper` is called. Already verified this solves the problem. >> This also means the work-around in `LazyObjectBinding` is no longer needed, >> which seems like a big win. >> >> 2) The second additional call originates from a different issue. When >> `ConditionalBinding` (which implements the `when` construct) sees its >> condition property changing, it always invalidates itself. This is however >> only necessary if the current cached value (if it was valid) differs from >> the current source value. To prevent an unnecessary invalidation, and the >> resulting revalidation calls that this will trigger, a simple check to see >> if the value actually changed before invalidating solves this problem. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix review comments Marked as reviewed by mstrauss (Committer). ------------- PR Review: https://git.openjdk.org/jfx/pull/1056#pullrequestreview-1388773707
