RFR: JDK-8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v9]
John Hendrikx
jhendrikx at openjdk.org
Fri Jun 9 01:33:27 UTC 2023
> This provides and uses a new implementation of `ExpressionHelper`, called `ListenerManager` with improved semantics.
>
> # Behavior
>
> |Listener...|ExpressionHelper|ListenerManager|
> |---|---|---|
> |Invocation Order|In order they were registered, invalidation listeners always before change listeners|(unchanged)|
> |Removal during Notification|All listeners present when notification started are notified, but excluded for any nested changes|Listeners are removed immediately regardless of nesting|
> |Addition during Notification|Only listeners present when notification started are notified, but included for any nested changes|New listeners are never called during the current notification regardless of nesting|
>
> ## Nested notifications:
>
> | |ExpressionHelper|ListenerManager|
> |---|---|---|
> |Type|Depth first (call stack increases for each nested level)|(same)|
> |# of Calls|Listeners * Depth (using incorrect old values)|Collapses nested changes, skipping non-changes|
> |Vetoing Possible?|No|Yes|
> |Old Value correctness|Only for listeners called before listeners making nested changes|Always|
>
> # Performance
>
> |Listener|ExpressionHelper|ListenerManager|
> |---|---|---|
> |Addition|Array based, append in empty slot, resize as needed|(same)|
> |Removal|Array based, shift array, resize as needed|(same)|
> |Addition during notification|Array is copied, removing collected WeakListeneres in the process|Tracked (append at end)|
> |Removal during notification|As above|Entry is `null`ed|
> |Notification completion with changes|-|Null entries (and collected WeakListeners) are removed|
> |Notifying Invalidation Listeners|1 ns each|(same)|
> |Notifying Change Listeners|1 ns each (*)|2-3 ns each|
>
> (*) a simple for loop is close to optimal, but unfortunately does not provide correct old values
>
> # Memory Use
>
> Does not include alignment, and assumes a 32-bit VM or one that is using compressed oops.
>
> |Listener|ExpressionHelper|ListenerManager|OldValueCaching ListenerManager|
> |---|---|---|---|
> |No Listeners|none|none|none|
> |Single InvalidationListener|16 bytes overhead|none|none|
> |Single ChangeListener|20 bytes overhead|none|16 bytes overhead|
> |Multiple listeners|57 + 4 per listener (excluding unused slots)|57 + 4 per listener (excluding unused slots)|61 + 4 per listener (excluding unused slots)|
>
> # About nested changes
>
> Nested changes are simply changes that are made to a property that is currently in the process of notifying its listeners. This all occurs on the same thread, and a nested change is nothing more th...
John Hendrikx has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 24 commits:
- Merge remote-tracking branch 'upstream/master' into
feature/nested-emission-with-correct-old-values
- Prevent removal of weak listeners during unlock
- Use an overridable method to store latest value
- Merge the recursive notification loop code
Made loop in ListenerList slightly more generic to allow merging the
logic in OldValueCachingListenerList and ListenerList; performance
impact seems minimal
- Small bug fix in OldValueCachingListenerList
- Added a test case to detect and avoid this problem
- Improve doc
- Move listener call code to ListListenerBase
- Improve generics on ObservableValueBase
- Improve generics
- Fix incorrect import
- ... and 14 more: https://git.openjdk.org/jfx/compare/8110f548...a9c488b9
-------------
Changes: https://git.openjdk.org/jfx/pull/1081/files
Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1081&range=08
Stats: 4355 lines in 39 files changed: 4192 ins; 7 del; 156 mod
Patch: https://git.openjdk.org/jfx/pull/1081.diff
Fetch: git fetch https://git.openjdk.org/jfx.git pull/1081/head:pull/1081
PR: https://git.openjdk.org/jfx/pull/1081
More information about the openjfx-dev
mailing list