I agree with Nir on this point. I don't see the middle ground as being
all that useful in practice, just more complicated, and the
specification change would be odd.
So I think we either go with the proposed change (and it would need to
be well-tested), or leave it as-is, where every new invalidation
listener validates the object being listened to. It would be an easy
call if it weren't for the concern about potentially breaking
applications that have baked in assumptions on the existing behavior.
-- Kevin
On 10/14/2021 9:39 AM, Nir Lisker wrote:
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ß <michaelstr...@gmail.com
<mailto:michaelstr...@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.rushfo...@oracle.com <mailto:kevin.rushfo...@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 <nlis...@gmail.com
<mailto:nlis...@gmail.com>
> > <mailto:nlis...@gmail.com <mailto:nlis...@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!dIduEJEBfgG0KXgohDc0mYSolHJl5R1cHpaCQ65ZZM577aoe65cYHAPYNhMJmmtCHBx7$>
> >
<https://urldefense.com/v3/__https://github.com/javafxports/openjdk-jfx/pull/110__;!!ACWV5N9M2RV99hQ!bIbtLsC0tg02c9a_lgKnM1Xta2USX8QkKRL4imOUSw8xshJsVquOeulplJR7dfEzQUf6$
<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
<hj...@xs4all.nl <mailto:hj...@xs4all.nl>
> > <mailto:hj...@xs4all.nl <mailto:hj...@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
> >
>
