Re: [hibernate-dev] Pooled Optimiser Improvements

Any arguments against merging the https://github.com/scottmarlow/hibernate-orm/commits/pooledOptimizer_5.x change to master + 5.x? I will create a jira for this change.

Any suggestions for how to specify in persistence.xml, that the PooledThreadLocalLoOptimizer should be used? We already have "hibernate.id.optimizer.pooled.prefer_lo", which I think can be true or false. Should we add another another similar property? Or perhaps allow "hibernate.id.optimizer.pooled.prefer_lo" to be set to "greedy thread local optimizer" or "pooled-lotl"? Something like: <property name="hibernate.id.optimizer.pooled.prefer_lo" value="pooled-lotl"/> On 12/15/2015 09:01 PM, Stuart Douglas wrote: > 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 >>> >> _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev