On Mon, 17 Feb 2025 15:37:56 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... > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix fix for regression :) <h3>Edge Case: A listener upon notification removes an earlier listener, then makes a nested change</h3> |ExpressionHelper|This PR|Specification| |---|---|---| |The removed listener is not notified of the initial or nested change|The removed listener is not notified of the initial or nested change|Leave unspecified| Opinion: as this operates on the same property on which you already have a listener, and the listener you removed must be something you have a reference to (and so probably created yourself), this case makes little sense in real world applications. We should leave it unspecified. <h3>Edge Case: A listener upon notification removes a later listener, then makes a nested change</h3> |ExpressionHelper|This PR|Specification| |---|---|---| |The removed listener is still notified of the initial change that triggered the initial notification as lists are copied|A removed listener is under no circumstances notified after removal|Leave unspecified| Opinion: as this operates on the same property on which you already have a listener, and the listener you removed must be something you have a reference to (and so probably created yourself), this case makes little sense in real world applications. We should leave it unspecified. <h3>Edge Case: A listener upon notification adds a listener, then makes a nested change</h3> |ExpressionHelper|This PR|Specification| |---|---|---| |The added listener is notified of the nested change, but not of the initial change|The added listener is not notified of the initial change, nor of the nested change|Leave unspecified| Opinion: as this operates on the same property on which you already have a listener, the added listener is superfluous; this case also makes little sense in real world applications. We should leave it unspecified. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-2671717626
