> 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 with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains five additional 
commits since the last revision:

 - Merge branch 'openjdk:master' into feature/remove-unnecessary-invalidatoins
 - Fix review comments
 - Address review comments
   
   - Added extra tests cases that count invalidations
   - Added explanatory comment
 - Fix review comments
 - Ensure ConditionalBinding only invalidates when absolutely needed
   
   Added additional test cases to verify this behavior

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

Changes:
  - all: https://git.openjdk.org/jfx/pull/1056/files
  - new: https://git.openjdk.org/jfx/pull/1056/files/c17bc639..8b0654ca

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=1056&range=04
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1056&range=03-04

  Stats: 17238 lines in 439 files changed: 9553 ins; 6115 del; 1570 mod
  Patch: https://git.openjdk.org/jfx/pull/1056.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1056/head:pull/1056

PR: https://git.openjdk.org/jfx/pull/1056

Reply via email to