On Tue, 4 Apr 2023 15:22:48 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
> 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 > 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 (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... I completely forgot about this PR, but it looks very interesting, especially about the nested events. If helpful, I can test this soon in a bigger application, especially for any regressions. And of course also review the code, but need more time for that. Tested with a big application, did not find any regression. All listeners still work as expected, tested especially a lot of 'if something is selected, this button should be enabled/disabled' and the like. I did not checked the code yet, just a little bit. One question: Should I test something special/do you see a case which could cause a problem here? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-1934958209 PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-1986169590
