On Tue, 15 Sep 2026 14:57:00 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 overhe...
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Implement notifying boolean

Future work that can be done after this goes in to improve performance for 
JavaFX listener management (beyond even what `ExpressionHelper` provides):

- Removal of listeners can be significantly faster by not always compacting the 
array as the implementation already supports the concept of tombstones (`null`s 
that are skipped)
  - An O(n) scan to find the listener to remove is still needed, but we can 
just make it a tombstone until some threshold is exceeded (@mstr2) -- this 
amortizes the normally mandatory array copy/shift
  - For locked lists that would mean that on `unlock` we don't always need to 
consolidate (if something was removed), but instead only consolidate when the 
number of tombstones exceeds some threshold
- We could support a very fast `indexOf` backed by an insertion ordered hash 
chain when the listener list exceeds a certain size (say >30 elements) -- that 
would address the O(n) scan needed to find the listener we want to remove

Combining the above two would result in almost instant listener removal 
(probably O(1) amortized).

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-5744982943

Reply via email to