Eager Evaluation of Invalidation Listeners

[Nir Lisker]

>
> 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
>

But why does an invalidation listener need to know the past state of the
property? It is only interested in the valid -> invalid transition. If the
property was invalid then the listener (in theory) shouldn't receive any
events anyway on subsequent invalidations. (I understand that you don't
justify this, I'm posing it as a general question.)

I suggest that we split the problem into 2: one is the case where the
property was valid when the listener was attached, and the other is when it
was invalid.
* A valid starting state. In this case attaching a listener shouldn't need
to do anything. A subsequent invalidation event will be sent regardless.
Currently, it is calling get() redundantly.
* An invalid starting state. In this case the documentation says that
nothing needs to happen, but get() is called anyway. Here, the difference
is that a subsequent invalidation event is sent in one case and not in the
other. The only way to advance here is to make a design decision on what
should happen, at least that's how I see it.

As to the implementation of a possible solution, suppose we add the isValid
method. Upon attaching an invalidation listener, if the property is valid,
we can skip the get() call. That solves the valid starting state issue. The
question is what to do if the property is not valid.

I also noticed an odd design choice in the implementation of properties:
the value field does not update if the property is bound, instead, the
result of the binding is returned and the value field holds an outdated
value (until the property is unbound). I would have thought that the value
would always be stored, and the binding will only be recomputed if the
value is invalid. If this was the case, there would not have been an eager
evaluation for the case when the property was valid upon attachment.
In a way, the implementation question here is if we expose the valid state
to let classes like ExpressionHelper check the valid state and decide what
to do, or if they call get() anyway and it's the property's job to not
recompute a valid value. I think we can defer this discussion to a later
stage.

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;
>

I understand the reasoning, but I think that sending an invalidation event
after the invalidation happened will be confusing and it's not what the
specs say, and might cause backward compatibility issues.

On Wed, Sep 1, 2021 at 12:28 AM John Hendrikx <hjohn at xs4all.nl> wrote:

>
>
> 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
>