[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: Eager Evaluation of Invalidation Listeners
From:       John Hendrikx <hjohn () xs4all ! nl>
Date:       2021-08-31 21:28:10
Message-ID: 63aa7a0f-17c1-a6de-eb10-eead56f5743a () xs4all ! nl
[Download RAW message or body]



On 31/08/2021 17:52, Nir Lisker wrote:
> Hi,
>
> This is a continuation of the discussion that stemmed from changing
> bidirectional bindings to use invalidation listeners instead of change
> listeners [1].
>
> The relevant issue in JBS is [2].
>
> I have looked at removing the eager evaluation when attaching an
> invalidation listener. I found that tests, for example, the
> method BooleanPropertyBaseTest#testAddingListenerWillAlwaysReceiveInvalidationEvent
> (only lines 428-432 are relevant) set the following requirement:
>
> 1. Start with a property in an invalid state.
> 2. Attach an invalidation listener.
> 3. Set a value which is different than the current one.
> Result: an invalidation event must be sent.
>
> This means that attaching an invalidation listener must validate the
> property, which is (at least currently) only possible by evaluating the
> value.
>
> This is in contrast to the comment in the JBS that invalidation listeners
> should not generally force eager evaluation. It is also in contrast to this
> line in the class doc of ObservableValue:
> "Implementations in this library mark themselves as invalid when the first
> invalidation event occurs. They do not generate any more invalidation
> events until their value is recomputed and valid again."
>
> So, either the requirement set by the tests is wrong, or invalidation
> listeners really do need to behave according to the test and the docs
> missed a practical requirement, or my analysis is wrong.

What I'm thinking is that perhaps the revalidation on registering a new 
InvalidationListener was done intentionally as there is no mechanism to 
query the current valid state of an observable (#isValid). So in order 
to make sure that a new interested invalidation listener does not miss 
the fact that a property was *already* invalid, the easiest solution 
might have been to revalidate it upon registration of a new listener -- 
then again, this does not communicate this fact, it only does so when 
the property changes again.

An interesting solution could be to send out an Invalidation event 
eagerly only to a newly registered listener if said property was invalid 
at the time of registration; this would inform the new listener that the 
property was already invalid but would not notify any existing listeners 
(as they already know the property to be invalid).

> There are more tests that fail, but the ones I looked at rely on the same
> concept. For
> example, Node_LocalToParentTransform_Test#shouldBeNotifiedWhenNodeTransforms()
> tests the Node#LazyTransformProperty property. Interestingly, this one is
> initialized with an invalid state, while Simple___Property are initialized
> as valid.
>
> Thoughts?

Perhaps we could consider the registration of an invalidation listener 
to be infrequent enough to be of little consequence, although I agree 
that the documentation and tests are not in agreement.

--John
[prev in list] [next in list] [prev in thread] [next in thread]