Re: [hibernate-dev] [ORM] Synchronization on AbstractLoadPlanBasedLoader

I'm confused. Sanne was talking about a completely different piece of code from optimizers. Maybe you are mixing this and the other current hibernate-dev discussion? On Mon, Dec 14, 2015 at 11:10 AM Vlad Mihalcea <mihalcea.vlad(a)gmail.com&gt; 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. > 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. > > Vlad > > On Mon, Dec 14, 2015 at 6:25 PM, Sanne Grinovero <sanne(a)hibernate.org&gt; > 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(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev >