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