On Thu, 26 Feb 2026 01:39:14 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:
>
> Remove unreliable unnecessary extra check
Hi,
From what I can see this type of behavior is inherent for an implementation in
which when event is "fired" it is delivered right in place to listeners like in
below pseudo code:
fireXXX()...
{
for each listener invoke onXXX(....)
}
It is common for all awt and swing listeners and may, in plenty of conditions
when nested firing occurs, lead to change in ordering of events. Yet, as far as
I investigated it, and I did put a lot of work into it more than decade ago, it
is a nature of in-place events firing.
As I do see it, the very crucial assumption behind in-place firing is: _"when
fireXXX returns, all listeners are notified"_. This is a strong indication of
state and data consistency and only in-place firing may do it. Like in this
pseudo code:
my_function()
{
fireXXX()
_here I can assume that all listeners reacted to XXX_
fireYYY()
_here I can assume that all listeners reacted to XXX and YYY_
}
I can assume it regardless if `my_function()` was invoked in response to event
or not.
This assumption is in a strict and conceptual conflict with _"when fireXXX is
invoked the events reach all listeners in the same order as in which events
were fired even if events are fired from inside of listener event handler"_.
Because now if my example function is invoked inside a listener those
assumptions may be false.
Any attempt to achieve the goal of delivering events in their order of
appearance will break the first assumption and it will lead to corruption of
existing programs. In many, many funny ways.
Also any reliance on _"if listener A was registered before listener B, then A
will get called before B"_ is a horribly bad idea. Please, do not even try to
suggest it. Simply do not. There is no way to ensure that ordering in any
system of some major complexity.
Any removing of "not necessary changes" in notification process is begging for
troubles. It did change, and it did changed again. Preventing that first change
from reaching the listeners is horrible idea, as listener is by definition
getting every change, not just "some".
In my practice, if I am worried about nested events and ordering problem, I do
consciously decide to "decouple" them. Instead of firing an event in place I
do, consciously, wrap them and delegate to the event pump (in case of awt -
with invokeLater). Then I do know that they will reach the destination in order
of firing, but I also do now that at any time the assumption that fireXXX
returning means that event is delivered to listeners is false.
This practice allows me to know, that when I invoke in-place fireXXX, I pay
with reordering of events for the state consistency. And when I delegate it, I
pay with guaranteed delay and wrapping for the benefit strict ordering. It is a
nice zero-one situation when I know what I can assume and what not, regardless
of what a listeners I notified decided to do.
My recommendation is: if You really plan to change in-place firing to some more
sophisticated algorithm **do not do it**. Not in a stable production code.
Recommend, inform, educate about possible side effect, alternatively add like
"orderedFireXXX" methods or something, but do not change such an essential
algorithm. As you can see above delegation is a simple method which allows
programmers using the library to decide what is important from them and what is
not. If they see problems with ordering delegation solves it. If however you
will build it in, they will have no way to ensure that if they have problems
with:
- lost "unnecessary" event, or;
- fireXXX returning without actually delivering information to listeners;
then they will have **no way** to work around it.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-4032502540
