[External] : Re: Eager Evaluation of Invalidation Listeners
Nir Lisker
nlisker at gmail.com
Thu Oct 14 16:39:23 UTC 2021
I disagree with this interpretation. Observable says
> Implementations in this library mark themselves as invalid when the first
> invalidation event occurs. They do not generate anymore invalidation events
> until their value is recomputed and valid again.
And ObservableValue says
An ObservableValue generates two types of events: ... An invalidation event
> is generated if the current value is not valid anymore.
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.
It's clear that the validity is with respect to the value, not the
listener. There is 1 value and it is either valid or invalid.
If we want to define validity on a per-listener basis, the docs would need
to be changed too. I don't know how much sense it makes practically because
I don't think anyone used them with this intention in mind. It could be a
middleground to bridge the current "negligence" with the stricter docs,
but it's a more fundamental change conceptually.
On Wed, Oct 6, 2021 at 8:02 PM Michael Strauß <michaelstrau2 at gmail.com>
wrote:
> The documentation of `Observable` states that "an implementation [...]
> may support lazy evaluation" and that "implementations [...] should
> strive to generate as few events as possible".
> This seems to me like it would be within spec to fire an invalidation
> event for every single change. It would also be within spec to fire
> redundant invalidation events only in certain scenarios (like adding a
> listener).
>
> The problem could also be approached from a different angle: what does
> it mean for a property to be "valid"?
> As implemented, the "valid" state is an opaque and unknowable
> implementation detail. We could re-define "valid" to mean: valid from
> the perspective of an InvalidationListener.
> A newly-added InvalidationListener wouldn't know that the property is
> invalid (and has no way of knowing), and can therefore reasonably
> assume that, from its perspective, the property is valid. It would
> receive an invalidation event when the property value is changed.
> From the perspective of pre-existing listeners, however, the property
> could well have been invalid all the time, so they won't receive an
> invalidation event.
>
> On Wed, Oct 6, 2021 at 2:16 AM Kevin Rushforth
> <kevin.rushforth at oracle.com> wrote:
> >
> > Given that the intention of InvalidationListener was to be a
> > light-weight way to listen to properties without causing a binding chain
> > to be revalidated, it does seem reasonable to me that the listener is
> > only fired on the valid --> invalid transition, which is the specified
> > behavior, even if the implementation doesn't currently do that in all
> cases.
> >
> > The two related questions then are:
> >
> > 1. Should adding an invalidation listener to property do an immediate
> > get(), which will ensure that the property is then valid? This will
> > force an eager evaluation of the property and "arm" the property to fire
> > an invalidation even the next time it is invalidated.
> >
> > 2. Should adding an invalidation listener to a currently invalid
> > property cause the listener to be called (either immediately or the next
> > time the object is invalidated)?
> >
> > I think the ideal answer to both questions is "no", although I share
> > John's concern that changing this behavior for InvalidationListeners
> > could break applications. So the question is: how likely do we think
> > that changing this behavior will break existing applications?
> >
> > I think it's something we can and should consider changing. Unless there
> > are serious concerns, I would support changing this behavior as part of
> > avoiding eager evaluation when using invalidation listeners. It would
> > need to be well tested (of course), and would need a CSR describing the
> > compatibility risk, and should probably get a release note.
> >
> > Any concerns in doing this?
> >
> > On the related question, I like the idea of nulling out the current
> > value when a property is bound.
> >
> > -- Kevin
> >
> >
> > On 9/11/2021 9:41 AM, Nir Lisker wrote:
> > > I think that we need your input on this to move it forward.
> > >
> > > On Fri, Sep 3, 2021 at 7:49 AM Nir Lisker <nlisker at gmail.com
> > > <mailto:nlisker at gmail.com>> wrote:
> > >
> > > so the value field should perhaps be nulled out when bound.
> > >
> > >
> > > There was a PR for something like that in the old repo:
> > > https://github.com/javafxports/openjdk-jfx/pull/110
> > > <
> https://urldefense.com/v3/__https://github.com/javafxports/openjdk-jfx/pull/110__;!!ACWV5N9M2RV99hQ!bIbtLsC0tg02c9a_lgKnM1Xta2USX8QkKRL4imOUSw8xshJsVquOeulplJR7dfEzQUf6$
> >
> > >
> > > On Fri, Sep 3, 2021 at 5:35 AM John Hendrikx <hjohn at xs4all.nl
> > > <mailto:hjohn at xs4all.nl>> wrote:
> > >
> > >
> > >
> > > On 02/09/2021 11:57, Nir Lisker wrote:
> > > > 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.)
> > >
> > > Strictly speaking, no, if users are using InvalidationListener
> > > correctly
> > > then this is definitely correct behavior. I'm not really
> > > advocating a
> > > change, and I'd even prefer that it be brought in line with the
> > > documentation.
> > >
> > > I think however that many users are not using it correctly and
> > > expect an
> > > invalidation event always the next time the value changes (and
> > > their
> > > listener will read that value, validating it again), making it
> > > act like
> > > a light-weight ChangeListener. I know that I probably have
> > > written code
> > > that made that assumption, and would in the past not even
> > > think twice
> > > about replacing a change with an invalidation listener or vice
> > > versa if
> > > that happened to be a better fit. Which is sort of what
> > > happened as well
> > > in the bidirectional binding PR, and the issue slipped past
> > > the author
> > > and two reviewers.
> > >
> > > > 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.
> > >
> > > True, the call to get is redundant in this case. Ugly too,
> > > calling get
> > > and discarding its result, while the intention is to force the
> > > property
> > > to become valid.
> > >
> > > > * 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.
> > >
> > > The docs are even more specific I think, they say no more
> > > events will be
> > > generated until it becomes valid -- it doesn't leave any
> > > option open
> > > that it could generate events if it wanted to.
> > >
> > > > 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).
> > >
> > > Yeah, that might not be a wise decision as that can lead to
> > > memory being
> > > referenced that users might expect to be freed. I didn't see
> > > anywhere
> > > defined what will happen to the value of the property when it
> > > is unbound
> > > again. The current implementation will keep its last value
> > > (during the
> > > unbind it will take the last value and assign it to its own
> value
> > > field), so the value field should perhaps be nulled out when
> > > bound.
> > >
> > > --John
> > >
> >
>
More information about the openjfx-dev
mailing list