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 <st...@hibernate.org> wrote: > Inline... > > On Wed, May 27, 2020 at 8:10 AM Sanne Grinovero <sa...@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@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
