[hibernate-dev] v6 and load-event

[Gail Badner]

Hi Steve,

Sorry, I have not read the entire thread carefully, so please disregard if
not relevant.

Would this provide functionality like what we discussed for an
OperationContext?

https://hibernate.atlassian.net/browse/HHH-10478

Thanks,
Gail

On Fri, May 29, 2020 at 4:21 AM Steve Ebersole <steve at hibernate.org> wrote:

> If/when it comes to needing that, I kind of like that continuation design.
> But I agree, Yoann is right - let's leave it off until needed or until we
> have specific requirements.
>
> Thanks everyone!
>
> On Fri, May 29, 2020 at 2:19 AM Sanne Grinovero <sanne at hibernate.org>
> wrote:
>
> > On Fri, 29 May 2020 at 07:22, Yoann Rodiere <yoann at hibernate.org> wrote:
> > >
> > > +1 not to add surround capability initially. Sounds better to start
> > simple and make things more complex when we actually need it :)
> >
> > Right. I didn't mean to raise additional requirements without having
> > investigated those tracing libraries - what I meant really is just to
> > raise awareness that we'll likely need to evolve it further when it
> > comes to finally implement such things.
> >
> > >
> > > Yoann Rodière
> > > Hibernate Team
> > > yoann at hibernate.org
> > >
> > >
> > > On Fri, 29 May 2020 at 07:25, Sanne Grinovero <sanne at hibernate.org>
> > wrote:
> > >>
> > >> Yes, I agree.
> > >>
> > >> On Thu, 28 May 2020, 22:11 Steve Ebersole, <steve at hibernate.org>
> wrote:
> > >>>
> > >>> Wanted to clarify -
> > >>>
> > >>> Regarding incremental addition of "surround listeners", so long as we
> > are all in agreement that this simply means there will be absolutely no
> > surround capability ***initially*** then I am fine with that.
> > >>>
> > >>> On Thu, May 28, 2020 at 4:10 PM Steve Ebersole <steve at hibernate.org>
> > wrote:
> > >>>>
> > >>>> Hm, the dynamic enable/disable stuff should be easy to handle, no?
> > Depends on what specific library you are thinking of and exactly how that
> > detail gets propagated to us.  At the end of the day, its really as
> simple
> > as protecting the creation of some of these objects with `if
> > (enabled)`-type checks.
> > >>>>
> > >>>> But again, if you have specific details in mind we can take a look.
> > >>>>
> > >>>> Also, I think it is not at all a good idea to even plan for
> > "different types of events".  In fact I'm fine with getting rid of
> > LoadEvent completely from that contract and simply directly passing the
> > information that is likely useful.  I mean at the end of the day a
> listener
> > for load events is going to be interested in the same set of information.
> > Yes, some will not need all of that information but that's not really a
> > concern IMO.  Especially if we inline the parameters and completely avoid
> > the event object instantiation
> > >>>>
> > >>>> Regarding incremental addition of "surround listeners", so long as
> we
> > are all in agreement that this simply means there will be absolutely no
> > surround capability then I am fine with that.
> > >>>>
> > >>>>
> > >>>> On Thu, May 28, 2020 at 3:55 PM Sanne Grinovero <
> sanne at hibernate.org>
> > wrote:
> > >>>>>
> > >>>>> On Thu, 28 May 2020 at 21:27, Steve Ebersole <steve at hibernate.org>
> > wrote:
> > >>>>> >
> > >>>>> > Any thoughts on this "continuation" approach?
> > >>>>>
> > >>>>> I love the pattern! Maybe we'll need also some ability to not
> capture
> > >>>>> the state for events which don't have any?
> > >>>>>
> > >>>>> I wonder if that implies we'll need two different event contracts:
> > one
> > >>>>> for the listeners which need state and one for those which don't;
> but
> > >>>>> I'm not eager to overcomplicate this.
> > >>>>>
> > >>>>> > Or maybe its just not important (yet) to handle "surround"
> > handling?
> > >>>>>
> > >>>>> I'm confident that integration with tracing libraries would be very
> > >>>>> useful and interesting to have - but indeed not having time to
> > >>>>> research it properly I'm a bit afraid that it might need further
> > >>>>> changes to reach excellent performance.
> > >>>>>
> > >>>>> For example one thing I remember is that with some libraries you're
> > >>>>> supposed to have the option to enable/disable the profiling options
> > >>>>> dynamically, and since there's an expectation of no overhead when
> > it's
> > >>>>> disabled this would need to imply having a way for the overhead of
> > >>>>> allocating space for the captured state to "vanish": this might be
> a
> > >>>>> bit more complicated, or need to be able to take advantage of JIT
> > >>>>> optimisations.
> > >>>>>
> > >>>>> So if we end up thinking that such event APIs need to be different
> > >>>>> depending on the need for state, perhaps indeed it's better to
> > >>>>> postpone the design of those with state to when someone has time to
> > >>>>> research an optimal integration with a tracing library. It might
> not
> > >>>>> be too hard, I just haven't explored it myself yet.
> > >>>>>
> > >>>>> Maybe let's do this incrementally, considering the "continuation"
> > >>>>> approach a next step?
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Sanne
> > >>>>>
> > >>>>> >
> > >>>>> > On Wed, May 27, 2020 at 9:27 AM Steve Ebersole <
> > steve at hibernate.org> wrote:
> > >>>>> >>
> > >>>>> >> Inline...
> > >>>>> >>
> > >>>>> >> On Wed, May 27, 2020 at 8:10 AM Sanne Grinovero <
> > sanne at hibernate.org> wrote:
> > >>>>> >>>
> > >>>>> >>> At high level I agree, just have 3 more thoughts:
> > >>>>> >>>
> > >>>>> >>> # Regarding the "swap" of information between listeners - could
> > that
> > >>>>> >>> even work? I might have misunderstood something, but wouldn't
> we
> > >>>>> >>> require listeners to run in some specific order for such
> > swapping to
> > >>>>> >>> work?
> > >>>>> >>
> > >>>>> >>
> > >>>>> >> This is why we allow control over the ordering of the registered
> > listeners.  And yes, that is and was a hokey solution.  Nothing changes
> > there really if that is why you are using load listener.
> > >>>>> >>
> > >>>>> >>
> > >>>>> >>>
> > >>>>> >>> # The "surround advice" you mention for e.g. timing seems very
> > >>>>> >>> interesting, especially as I'd love us to be able to integrate
> > with
> > >>>>> >>> tracing libraries - but these would need to be able to
> co-relate
> > the
> > >>>>> >>> pre-load event with some post-load event. How would that work?
> > I'd
> > >>>>> >>> expect these to need having a single listener implementation
> > which
> > >>>>> >>> implements both PreLoadEventListener and PostLoadEventListener,
> > but
> > >>>>> >>> also they'll likely need some capability to store some
> > information
> > >>>>> >>> contextual to the "event".
> > >>>>> >>
> > >>>>> >>
> > >>>>> >> I was just thinking through this one as well.  My initial
> thought
> > was exactly what you proposed - some combination of pre/post listener
> with
> > some ability to store state between.  But that gets ugly.
> > >>>>> >>
> > >>>>> >> Another option I thought about is easier to illustrate, but
> > basically works on the principle of "continuation" many surround advice
> > solutions are based on:
> > https://gist.github.com/sebersole/142765fe2417492061e92726e7cb6bd8
> > >>>>> >>
> > >>>>> >> I kept the name LoadEventListener there, but since it changes
> the
> > contract anyway I'd probably rename this to something like
> > SurroundLoadEventListener
> > >>>>> >>
> > >>>>> >>
> > >>>>> >>>
> > >>>>> >>> # To clarify on my previous comment regarding why I'd consider
> > having
> > >>>>> >>> an actual Event class more maintainable:
> > >>>>> >>> Sure we won't have inline classes widely used for a while, but
> I
> > >>>>> >>> prefer planning for the long term - also we could start using
> > them
> > >>>>> >>> very soon via multi-release jars, which would simply imply that
> > users
> > >>>>> >>> on newer JDKs would see more benefits than other users.
> > >>>>> >>> But especially, such event instances are passed over and over
> > across
> > >>>>> >>> many methods; so in terms of maintenance and readability, such
> > methods
> > >>>>> >>> would need to pass many parameters rather than one: the example
> > made
> > >>>>> >>> above is oversimplifying our use.  Also while I understand it's
> > >>>>> >>> unlikely, having a "cheap" event objects makes it easier to
> > change the
> > >>>>> >>> exact types being passed on.
> > >>>>> >>> BTW stack space is cheap but forcing many references to be
> > passed when
> > >>>>> >>> one single reference could do might also have some performance
> > >>>>> >>> implications since these are passed many times - I've never
> > tested
> > >>>>> >>> this scientifically though :)   Inline objects would typically
> be
> > >>>>> >>> allocated on the stack as well, but they don't force the JVM to
> > do so.
> > >>>>> >>>
> > >>>>> >>>
> > >>>>> >>> Also while I said that it's unlikely we want to change those
> > types,
> > >>>>> >>> the very coming of inline types might actually encourage us to
> > make
> > >>>>> >>> changes in this area, even though these events have been stable
> > for
> > >>>>> >>> years; for example "String entityName" seems like an excellent
> > >>>>> >>> candidate to become "EntityName typeIdentifier" - and then
> allow
> > us to
> > >>>>> >>> improve the persister maps, which have been a bottleneck in the
> > past.
> > >>>>> >>> So sure we could remove them and just pass parameters, we'd
> just
> > need
> > >>>>> >>> to change more code if such a situation arises - I'm just
> > highliting
> > >>>>> >>> the drawbacks for our consideration, not recommending against
> it
> > :)
> > >>>>> >>
> > >>>>> >>
> > >>>>> >> Maybe its simply a difference of wording, but to me none of this
> > validates how keeping an event class is more maintainable.  If you want
> to
> > say that eventually the overhead of having an actual event class will be
> > less, ok, but that's different.
> > >>>>> >>
> > >>>>> >> For sure though we'd have lots of uses for in-line value types
> > throughout the code base.  Just not sure this really an argument for
> > keeping the event impl in-and-of-itself.
> > >>>>> >>
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev