[hibernate-dev] [ORM] Synchronization on AbstractLoadPlanBasedLoader

[Steve Ebersole]

Sanne, you just described the general approach of "short naming" for config
options overall :)



On Mon, Dec 14, 2015 at 6:28 PM Sanne Grinovero <sanne at hibernate.org> wrote:

> On 14 December 2015 at 17:10, Vlad Mihalcea <mihalcea.vlad at gmail.com>
> wrote:
> > Hi,
> >
> > We really need to test it thoroughly because the current pooled optimizer
> > are reasonably fast especially when used with a database sequence.
> > The table generators are slow because of the row-level locking, so I
> won't
> > include those in this discussion.
> >
> > What strikes me is the synchronized block which might cause contention we
> > didn't have before.
>
> Yes, but that's actually a sign I'm very happy with ;-)
> The performance team from Red Hat has been helping a lot the past few
> years, and Hibernate 5 incorporates many improvements based on both
> their patches and their regular feedback.
> They are now able to scale it up like never before, and so doing these
> synchronization points are now "bottlenecks", while they would
> previously not be because of other things slowing it down.
>
> > I'd also vote for a new optimizer instead of modifying the pooled or the
> > pooled-lo ones.
> > The current optimizer are quite easy to grasp, and, if we are to add a
> > high-performance one, I think a new implementation is better suited for
> this
> > task.
>
> Maybe, if we're considering the new ones experimental, but these
> changes are relatively simple and aren't changing the fundamental
> design.
>
> I am not sure how far it helps end users to have a long list of
> choices, when to understand which one to use one has to describe the
> implementation in detail.
>
> I would rather use some short-hand names which describe the different
> requirements one might have - for example someone might need
> monotonically increasing sequences, while someone else might be happy
> with non-strictly monotonical sequences if they can get better
> performance out of it.
> >From the different names, we should be able to activate the right
> implementations; for example the choice between an highly optimized
> implementation which would only work for non-multitenant use cases
> like Steve suggested could be an implementation detail: we'd opt to
> use that implementation internally when someone picks "pooled-lo" and
> happens to not be using multitenancy.
>
> But of course power users are always right, so if someone configures
> an implementation explicitly by naming it by fully qualified
> classname, then we should of course assume that the user knows best
> (although not as much as to not validate - for example again - that
> he's using a non-multitenant one with multitenancy).
>
> Thanks,
> Sanne
>
> >
> > Vlad
> >
> > On Mon, Dec 14, 2015 at 6:25 PM, Sanne Grinovero <sanne at hibernate.org>
> > wrote:
> >>
> >> Hi all,
> >> while reviewing an improvement by Stale about reducing
> >> synchronization, I'm having the impression that the synchronization
> >> could be completely removed.
> >>
> >> But there's a comment warning me against that, so for sake of safety
> >> I'm merging the improvement without risking going too far:
> >>
> >>  // synchronized to avoid multi-thread access issues; defined as
> >> method synch to avoid
> >>  // potential deadlock issues due to nature of code.
> >>
> >> I tried to figure what "potential deadlock" it's referring to, but I'm
> >> having the impression the comment might be outdated. So I've reduced
> >> the contention to the only include the code block about which I'm not
> >> confident.
> >> By looking into git history, it seems the comment isn't related to any
> >> specific fix but was included already when this class was first
> >> created.
> >>
> >> Would someone be able to point out what is the issue this is protecting
> >> against?
> >>
> >> That should allow us to provide an even better patch, although I'll
> >> apply the safe one for now so to at least have the benefits already
> >> when wrapping of result-sets is disabled.
> >>
> >> thanks,
> >> Sanne
> >> _______________________________________________
> >> hibernate-dev mailing list
> >> hibernate-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>