[hibernate-dev] v6 and load-event

[Steve Ebersole]

Any thoughts on this "continuation" approach?

Or maybe its just not important (yet) to handle "surround" handling?

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