[hibernate-dev] Pooled Optimiser Improvements
Steve Ebersole
steve at hibernate.org
Wed Dec 16 11:04:19 EST 2015
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 <smarlow at 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" <smarlow at redhat.com>
> >>> To: "Steve Ebersole" <steve at hibernate.org>, "Stuart Douglas" <
> sdouglas at redhat.com>, hibernate-dev at 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 at redhat.com
> >>>>>>> <mailto:smarlow at 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 at redhat.com
> >>>>>>> <mailto:smarlow at redhat.com>
> >>>>>>> >> <mailto:smarlow at redhat.com <mailto:smarlow at 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 at lists.jboss.org
> >>>>>>> <mailto:hibernate-dev at lists.jboss.org>
> >>>>>>> <mailto:hibernate-dev at lists.jboss.org
> >>>>>>> <mailto:hibernate-dev at lists.jboss.org>>
> >>>>>>> >> >
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>>>>>> >> >
> >>>>>>> >> _______________________________________________
> >>>>>>> >> hibernate-dev mailing list
> >>>>>>> >> hibernate-dev at lists.jboss.org
> >>>>>>> <mailto:hibernate-dev at lists.jboss.org>
> >>>>>>> <mailto:hibernate-dev at lists.jboss.org
> >>>>>>> <mailto:hibernate-dev at lists.jboss.org>>
> >>>>>>> >>
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>>>>>> >>
> >>>>>>> > _______________________________________________
> >>>>>>> > hibernate-dev mailing list
> >>>>>>> > hibernate-dev at lists.jboss.org
> >>>>>>> > <mailto:hibernate-dev at lists.jboss.org>
> >>>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>>>>>> >
> >>>>>>>
> >>>>>> _______________________________________________
> >>>>>> hibernate-dev mailing list
> >>>>>> hibernate-dev at lists.jboss.org
> >>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>>>>>
> >>>>> _______________________________________________
> >>>>> hibernate-dev mailing list
> >>>>> hibernate-dev at lists.jboss.org
> >>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>>>>
> >>>> _______________________________________________
> >>>> hibernate-dev mailing list
> >>>> hibernate-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>>>
> >>>
> > _______________________________________________
> > hibernate-dev mailing list
> > hibernate-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>
More information about the hibernate-dev
mailing list