On Tue, 4 Apr 2023 15:22:48 GMT, John Hendrikx <[email protected]> wrote:
> This provides and uses a new implementation of `ExpressionHelper`, called > `ListenerManager` with improved semantics. > > See also #837 for a previous attempt which instead of triggering nested > emissions immediately (like this PR and `ExpressionHelper`) would wait until > the current emission finishes and then start a new (non-nested) emission. > > # 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 > WeakListeners in the process|Appended when notification finishes| > |Removal during notification|As above|Entry is `null`ed (to avoid moving > elements in array that is being iterated)| > |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... This pull request has now been integrated. Changeset: 8778f6e0 Author: John Hendrikx <[email protected]> URL: https://git.openjdk.org/jfx/commit/8778f6e0cb351d37d572208efb10609c55d66f82 Stats: 8898 lines in 67 files changed: 7162 ins; 1432 del; 304 mod 8290310: ChangeListener events are incorrect or misleading when a nested change occurs 8334429: ObservableValue docs wrongly state that reference equality is used to detect and report changes Reviewed-by: mhanl, angorya, kcr, mstrauss, nlisker ------------- PR: https://git.openjdk.org/jfx/pull/1081
