On Sun, 16 Apr 2023 12:34:44 GMT, John Hendrikx <[email protected]> wrote:
>> I agree with you there, and I've been looking what would be a good way to
>> achieve this. I will take another look soon. My primary concern is that
>> this is a somewhat critical path, and I would want to ensure that it doesn't
>> cause too much performance regressions (I've already been optimizing all of
>> this code with the help of a JMH test)
>
> While looking that code over to see if it could be merged without impacting
> the general case, I discovered a small bug in the OldValueCaching version.
> After I fixed it, the code was even more similar than it was already. The
> only different still is the fact that the latest value must be kept track of
> whenever ObservableValue#getValue is called.
>
> I've now added an extra parameter to the generic version to allow for storing
> the latest value when it is queried (and not storing it if it's not needed).
> This seems to have a minimal performance impact only, so I think the trade
> off is acceptable.
Have you considered adding a method like `void valueUpdated(T value) {}` to
`ListenerList`? This will require `ListenerList` to have a type variable `T`
(which `OldValueCachingListenerList` adds anyway).
This method could then be called instead of
`latestValueTracker.accept(newValue)`, and `OldValueCachingListenerList` can
override it and store the value. The advantage of that would be that we don't
need the `latestValueTracker` field.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r1167979736
