[hibernate-dev] v6 and load-event

Fri Jun 5 15:45:23 EDT 2020

Load event handling does not have "anything" parameters.  Am I
understanding you correctly?

On Fri, Jun 5, 2020, 11:42 AM Gail Badner <gbadner at redhat.com> wrote:

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