Re: [hibernate-dev] Pooled Optimiser Improvements

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" <smarlow(a)redhat.com&gt; > To: "Steve Ebersole" <steve(a)hibernate.org&gt;, "Stuart Douglas" <sdouglas(a)redhat.com&gt;, hibernate-dev(a)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 <smarlow(a)redhat.com >>>>> <mailto:smarlow@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 >>>>> >> <smarlow(a)redhat.com >>>>> <mailto:smarlow@redhat.com> >>>>> >> <mailto:smarlow@redhat.com <mailto:smarlow@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...stuartwdougla... >>>>> >> > >>>>> >> > 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(a)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(a)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(a)lists.jboss.org >>>>> > <mailto:hibernate-dev@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 >>>> >>> _______________________________________________ >>> 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 >> >