Re: [hibernate-dev] Pooled Optimiser Improvements

On 12/16/2015 09:07 AM, Scott Marlow wrote: > 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. HHH-10381 > > 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 > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev