With my original patch the intention was that that the thread local blocks were smaller than the incrementSize, so not every thread local allocation would require a DB call. Your patch changes that approach but I don't think it actually matters that much, the overall performance should still be similar, and it has the advantage of not needed an extra configuration value.
Stuart ----- Original Message ----- > From: "Scott Marlow" <smar...@redhat.com> > To: "Steve Ebersole" <st...@hibernate.org>, "Stuart Douglas" > <sdoug...@redhat.com>, hibernate-dev@lists.jboss.org > Sent: Wednesday, 16 December, 2015 10:15:49 AM > Subject: Re: [hibernate-dev] Pooled Optimiser Improvements > > https://github.com/scottmarlow/hibernate-orm/commits/pooledOptimizer_5.x > is looking more correct now, if others want to look at that. > > On 12/15/2015 07:58 PM, Scott Marlow wrote: > > > > > > On 12/15/2015 05:58 PM, Scott Marlow wrote: > >> > >> > >> On 12/15/2015 05:40 PM, Scott Marlow wrote: > >>> I changed the new test methods a bit. [2] seems to be passed the tests > >>> but I am not understanding how PooledThreadLocalLoOptimizer should > >>> coordinate with the AccessCallback to allocate the next chunk of > >>> sequence numbers. > >>> > >>> We seem to be able to call AccessCallback.getNextValue() to get the next > >>> available sequence number but how do we reserve a block of 5000 sequence > >>> ids? Am I supposed to call callback.getNextValue() an extra time to get > >>> a range of values? Is there a separate database transaction that is > >>> used by the AccessCallback.getNextValue() calls? I'm missing something. > >> > >> Thinking more about this, I assume that AccessCallback.getNextValue() is > >> operating under a database transaction that we are probably ending > >> before AccessCallback.getNextValue() returns. It also sounds like the > >> database table is tracking the "lo" value, as mentioned in the > >> PooledLoOptimizer. This implies that only the application layer knows > >> what the range is. This seems like an important dependency to understand. > >> > >> Make sense? > > > > http://in.relation.to/2007/04/10/new-323-hibernate-identifier-generators > > seems to explain how increment_size is used. Since the user is already > > configured that, will look into switching to that for > > PooledThreadLocalLoOptimizer. > > > >> > >>> > >>> Note that [2] also includes a test change to comment out a few lines in > >>> SchemaUpdateDelimiterTest, due to the compiler error that I am seeing in > >>> intellij. Will need to remember to remove that change. > >>> > >>> [2] > >>> https://github.com/scottmarlow/hibernate-orm/commits/pooled-optimiser-hack-2 > >>> > >>> On 12/15/2015 12:36 PM, Steve Ebersole wrote: > >>>> Those tests tend to assert the increments. We seem to agree that this > >>>> ThreadLocal one can skip gaps of values. I'd look there first. > >>>> > >>>> > >>>> On Tue, Dec 15, 2015 at 11:34 AM Scott Marlow <smar...@redhat.com > >>>> <mailto:smar...@redhat.com>> wrote: > >>>> > >>>> I'm trying to move the optimizer to PooledThreadLocalLoOptimizer > >>>> [1]. > >>>> We are currently failing some new unit tests, which are cloned > >>>> from > >>>> existing PooledLoOptimizer tests which might be part of the > >>>> problem. > >>>> > >>>> Scott > >>>> > >>>> [1] > >>>> > >>>> https://github.com/scottmarlow/hibernate-orm/tree/pooled-optimiser-hack > >>>> > >>>> On 12/14/2015 10:12 PM, Scott Marlow wrote: > >>>> > > >>>> > > >>>> > On 12/11/2015 09:30 AM, Steve Ebersole wrote: > >>>> >> It's hard to say without understanding the scenario where you > >>>> are seeing > >>>> >> this as a problem. I have some guesses as to what may be the > >>>> problem, > >>>> >> but without understanding more about why you see this as a > >>>> problem in > >>>> >> the first place it is hard to give you an answer. For > >>>> >> example, > >>>> I wonder > >>>> >> if for environments not using multi-tenancy whether the > >>>> >> recent > >>>> changes > >>>> >> for the generators to support multi-tenancy might be the > >>>> culprit. If > >>>> >> that is the case, and those changes are in fact the > >>>> >> underlying > >>>> cause of > >>>> >> the perf issues you see then I think there is actually a > >>>> >> better > >>>> >> solution. But again, its hard to say unless we understand > >>>> >> the > >>>> reason > >>>> >> this "shows up" as a perf problem for you. > >>>> > > >>>> > As best as I can tell from looking at the current > >>>> > PooledLoOptimizer, > >>>> > versus the proposed change (to have a chunk of ids per > >>>> > thread), > >>>> we went > >>>> > from accessing a contented lock, to instead using per thread > >>>> > memory > >>>> > (eliminating the contended lock on > >>>> > PooledLoOptimizer.generate()). > >>>> > > >>>> >> > >>>> >> Until we hear more I think at this stage I'd vote for a > >>>> >> separate > >>>> >> optimizer. And maybe even not one that is upstream. > >>>> >> > >>>> >> Also I agree with Scott that I am VERY leery of not cleaning > >>>> >> up a > >>>> >> ThreadLocal. > >>>> > > >>>> > My mistake, as Stuart pointed out, the TL is not static, so we > >>>> shouldn't > >>>> > introduce any leaks. > >>>> > > >>>> >> > >>>> >> On Fri, Dec 11, 2015 at 7:55 AM Scott Marlow > >>>> >> <smar...@redhat.com > >>>> <mailto:smar...@redhat.com> > >>>> >> <mailto:smar...@redhat.com <mailto:smar...@redhat.com>>> > >>>> >> wrote: > >>>> >> > >>>> >> Should this be a specialized pooled optimizer that is > >>>> >> only > >>>> used in > >>>> >> environments that do not suffer from leaving the > >>>> ThreadLocal around > >>>> >> after the application is undeployed? In other words, > >>>> >> the > >>>> expectation is > >>>> >> that classloader leaks with this pooled optimizer are > >>>> expected (e.g. > >>>> >> user must restart the jvm to really undeploy the > >>>> >> application > >>>> >> completely). > >>>> >> > >>>> >> I am thinking that there are at least three typical > >>>> >> situations: > >>>> >> > >>>> >> 1. Applications are deployed in Java standalone > >>>> >> edition. > >>>> Generally, > >>>> >> when the app undeploys the jvm is shutting down. > >>>> >> > >>>> >> 2. Applications are deployed as part of some container > >>>> (e.g. an EE > >>>> >> server) and the Hibernate jars are on the global > >>>> classloader path (or > >>>> >> something like that). On each shared container thread, > >>>> there would be > >>>> >> one Optimizer for all deployed applications. I wonder > >>>> >> if > >>>> instead, we > >>>> >> would want one Optimizer instance per Hibernate > >>>> >> SessionFactory > >>>> >> associated with the many container threads? > >>>> >> > >>>> >> 3. Applications are deployed as part of some container > >>>> (e.g. an EE > >>>> >> server) and the Hibernate jars are deployed with the > >>>> application. The > >>>> >> ThreadLocals are associated with threads that are shared > >>>> >> by > >>>> different > >>>> >> deployed applications. The application classloader > >>>> >> contains the > >>>> >> Hibernate classes. Each deployed application has its own > >>>> Optimizer > >>>> >> threadlocal. On each shared container thread, there > >>>> >> would > >>>> be one > >>>> >> Optimizer per application (since each application has > >>>> >> its > >>>> Optimizer TL). > >>>> >> Like (2), there would be sharing of the same > >>>> >> Optimizer > >>>> with the many > >>>> >> application session factories. Should we instead have > >>>> >> an > >>>> optimizer per > >>>> >> session factory? > >>>> >> > >>>> >> Scott > >>>> >> > >>>> >> On 12/10/2015 11:31 PM, Stuart Douglas wrote: > >>>> >> > Hello, > >>>> >> > > >>>> >> > I have been working on a change to the pooled > >>>> >> > optimizer > >>>> that we > >>>> >> have been seeing good performance results with. > >>>> >> Basically > >>>> it hands > >>>> >> out blocks of ID's to a thread local, rather than having > >>>> >> every > >>>> >> thread contend on the lock every time an ID is required. > >>>> >> > > >>>> >> > > >>>> >> > >>>> > >>>> https://github.com/hibernate/hibernate-orm/compare/master...stuartwdouglas:pooled-optimiser-hack > >>>> >> > > >>>> >> > What would I need to do to get a change like this in? > >>>> >> > In > >>>> particular: > >>>> >> > > >>>> >> > - Does it need to be a new type of optimizer, or is > >>>> modifying the > >>>> >> existing one like I have done OK? > >>>> >> > - How should it be configured? > >>>> >> > > >>>> >> > I am happy to do up a PR for this, but I am just not > >>>> really sure > >>>> >> what would be required to get it to a point where it > >>>> >> would be > >>>> >> acceptable for inclusion. > >>>> >> > > >>>> >> > Stuart > >>>> >> > _______________________________________________ > >>>> >> > hibernate-dev mailing list > >>>> >> > hibernate-dev@lists.jboss.org > >>>> <mailto:hibernate-dev@lists.jboss.org> > >>>> <mailto:hibernate-dev@lists.jboss.org > >>>> <mailto:hibernate-dev@lists.jboss.org>> > >>>> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>>> >> > > >>>> >> _______________________________________________ > >>>> >> hibernate-dev mailing list > >>>> >> hibernate-dev@lists.jboss.org > >>>> <mailto:hibernate-dev@lists.jboss.org> > >>>> <mailto:hibernate-dev@lists.jboss.org > >>>> <mailto:hibernate-dev@lists.jboss.org>> > >>>> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>>> >> > >>>> > _______________________________________________ > >>>> > hibernate-dev mailing list > >>>> > hibernate-dev@lists.jboss.org > >>>> > <mailto:hibernate-dev@lists.jboss.org> > >>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>>> > > >>>> > >>> _______________________________________________ > >>> hibernate-dev mailing list > >>> hibernate-dev@lists.jboss.org > >>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>> > >> _______________________________________________ > >> hibernate-dev mailing list > >> hibernate-dev@lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
