A few more code changes are needed for the optimiser optimiser.
On 12/16/2015 11:04 AM, Steve Ebersole wrote: > Well keeping in mind that IMO this should be a separate optimizer (I > know I won't be the only one to be leery of ThreadLocals here), users > should be able to specify this one explicitly at the > generator-definition site. > > Of course not all use cases allow explicitly specifying this, which is > sort of what you are getting at. > `hibernate.id.optimizer.pooled.prefer_lo` was the initial attempt at > such use cases based on the original optimizers. At the end of the day > we are really just trying to determine the default optimizer to use when > one was not explicitly specified. Previously this was just a choice > between POOLED and POOLED_LO (hence the boolean). Stuart is bringing up > a new suggestion in this decision point. Really I think the best option > is simply to allow the user to specify the default pooled optimizer they > want to use : POOLED, POOLED_LO, POOLED_LOTL (fugly name). > > I see your latest reply now Scott. And I don't think that changing a > previously boolean setting to now accept Optimizer names is the correct > solution. I think we leave hibernate.id.optimizer.pooled.prefer_lo as > is, although possibly deprecate it. We'd add a new config setting for > this: `hibernate.id.optimizer.pooled.preferred`. If not set we fallback > to `hibernate.id.optimizer.pooled.prefer_lo` and what we do today. > > > > On Wed, Dec 16, 2015 at 8:24 AM Scott Marlow <smar...@redhat.com > <mailto:smar...@redhat.com>> wrote: > > > > 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" <smar...@redhat.com > <mailto:smar...@redhat.com>> > >>> To: "Steve Ebersole" <st...@hibernate.org > <mailto:st...@hibernate.org>>, "Stuart Douglas" <sdoug...@redhat.com > <mailto:sdoug...@redhat.com>>, hibernate-dev@lists.jboss.org > <mailto: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> > >>>>>>> <mailto: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>> > >>>>>>> >> <mailto: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>> > >>>>>>> <mailto: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>> > >>>>>>> <mailto: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 > <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 <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
