6:39 a.m.
The overall concept with event+listeners initially was that listeners would
"collaborate" which each other with the event as a token for common
information. The "result" of the operation was made part of the event to
facilitate this design - the idea being that one listener might swap the
result, etc. But in practice that does not happen essentially ever that I
have seen.
But if we split this idea of handler from listener, then LoadEvent is no
longer strictly necessary.
How we handle PreLoadEvent and PostLoadEvent is another question. Maybe
here it is best to maintain the stability of the API, as Yoann points out,
to minimize the work for upgrading for users who supply custom events.
Assuming we do keep those pre/post events as-is (dropping
PreLoadEventListener and PostLoadEventListener from my gist), then I
completely agree about delaying the creation of those events until we need
them... and reusing those created instances.
The "swap" use case I mentioned earlier is something important to keep in
mind. Do we feel like we need such a feature? Keeping the pre/post load
stuff working as-is would allow for that. Otherwise we'd need to decide if
that is important and decide how to allow it in the new contract.
Sounds like we all agree dropping LoadEvent and LoadEventListener in favor
of LoadEventHandler is a good idea.
How does everyone feel about PreLoadEventListener / PostLoadEventListener
versus the existing pre/post-load event and listeners? I think it's also a
good idea to understand the ways a user might want to use a listener and
advice based on what they want to achieve:
- pre-event notification
- post-event notification
- pre-event validation
- post-event validation
- result swapping
- "surround advice" - pre/post combo (timing, etc)
- others?
I think those use-cases fit in with either approach. What do y'all think?
On Wed, May 27, 2020 at 2:18 AM Yoann Rodiere <yoann(a)hibernate.org> wrote:
Hi,
> I think that's a great idea
Same here!
Another advantage is we separate the "SPI"/internal interface (Handler)
from the API that can be implemented by users (listeners).
That could be a great help moving forward to evolve Hibernate ORM without
breaking APIs.
> but I wonder about the readability of the code.
I was thinking that it was actually *more* readable. Maybe it's just a
matter of taste, but this:
Object loaded = handler.load( param1, param2, ... )
Seems considerably more straightforward than this:
LoadEvent event = new LoadEvent( param1, param2, ... )
for (LoadListener listener: listeners) {
listener.onLoad( event );
}
Object loaded = event.getLoaded();
Especially if you consider that with the handler, the implementation of
loading is just one CTRL+click away.
With listeners you have to go through all the listener implementations and
find which one actually implements loading.
Not a problem with load since there's only one implementation, but I know
I've had trouble with other events in the past.
The devil is in the details, though. I admit it could get more complex if
multiple return values are necessary, and maybe there are other problems
with this approach that we can't see right away.
But still, I think it's worth a try.
> With "inline types" coming soon to the JDK, the event object
types could be great candidates to be converted into inline?
Aren't you talking about JDKs that we won't be able to use as the baseline
for Hibernate ORM for a few years at least? If so, Steve's change seems
worthwhile, if only in the meantime.
Yoann Rodière
Hibernate Team
yoann(a)hibernate.org
On Tue, 26 May 2020 at 20:17, Sanne Grinovero <sanne(a)hibernate.org> wrote:
> Hi Steve,
>
> looks like you're getting rid of the "event object"? No more events to
> be allocated?
>
> I think that's a great idea, but I wonder about the readability of the
> code. With "inline types" coming soon to the JDK, the event object
> types could be great candidates to be converted into inline?
>
> If we used the inline types for such things I'm confident that
> allocation would improve significantly; it's undeniable that not
> having anything-at-all would be even better - but perhaps it could be
> a better tradeoff in consideration of maintainability.
>
> I also noticed that it's quite possible that some "list of
> eventlisteners" for a specific event type could be empty: no listeners
> at all.
> So an aspect that could be worth exploring while thinking about the
> event system design, is to avoid creating en event in such cases;
> there's an example of this approach already in
> org.hibernate.internal.SessionImpl#internalClear : the signature I had
> to use is a bit puzzling, but it manages to avoid allocating the event
> (unless needed) and also avoids allocating the iterator on the
> listeners.
>
> For example PostLoad events by default have a single listener, but
> looking more closely I believe we could avoid registering that single
> listener depending on configuration.
>
> Thanks,
> Sanne
>
>
>
> On Tue, 26 May 2020 at 16:00, Steve Ebersole <steve(a)hibernate.org> wrote:
> >
> > Historically we made a terrible design mistake with the event system as
> a
> > whole. This has both a confusing design impact and a very real
> performance
> > impact. The main problem is the smashing together of things that handle
> > events and things that listen to such events.
> >
> > In working on a problem in v6 I have come to a point where fixing this
> > design flaw for load events specifically would be a huge help. So I'm
> > going to make a proposal for how to specifically change this for load
> event
> > handling. The plan at the moment is to not make similar changes to the
> > other event types for now, though I certainly would like to keep them
> all
> > in mind with regard to the overall design for if/when we can get to the
> > others.
> >
> > So I'd love feedback specifically regarding (1) general design
> considering
> > other event types and (2) issues/concerns specific to load event type.
> >
> > I created a simple example at
> > https://gist.github.com/sebersole/2a1c3ac010a166fc91e62b088179678d
> >
> > Thanks!
> > _______________________________________________
> > hibernate-dev mailing list
> > hibernate-dev(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>
>