commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r1051649 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Date Wed, 22 Dec 2010 00:00:54 GMT
On Tue, Dec 21, 2010 at 6:01 PM, sebb <sebbaz@gmail.com> wrote:

> On 21 December 2010 22:44, Mark Thomas <markt@apache.org> wrote:
> > On 21/12/2010 21:41, Simone Tripodi wrote:
> >> Hi guys,
> >> thanks for the quick feedbacks, I marked them as synchronized just
> >> because in one of the last threads we agreed to make them
> >> synchronized.
> >
> > I can't find a that particular decision in the archives. Can you provide
> > a reference please.
> >
> >> If there is the need to make class fields volatile instead, it's fine
> >> by me but I suggest to discuss about it in another thread to involve
> >> everybody in the decision.
> >
> > I don't see the need for another thread. All concerned should be reading
> > this one already.
>
> +1
>
> > My concern with any sync is that it runs the risk of introducing
> > bottlenecks and bottlenecks are the number one performance issue with
> > the current DBCP/POOL combo - particularly on multi-core systems that
> > make lots of calls borrow/return. Ideally there won't be any syncs on
> > the borrow/return code path.
> >
> > The current pool impl grabs a copy of the current config attributes
> > (which are volatile) it cares about at the beginning of the method and
> > uses them for the remainder of the method. If the config is changed
> > whilst the method is executing - those changes have no effect on that
> > particular execution.
>

OK enough - not completely true in current impl  (borrowObject,  for example
snaps only whenExhaustedAction and maxWait) but I think the performance
benefit is way more important than perfect semantics.  An intermediate
alternative would be to introduce a different lock for the config
properties.  That would guarantee integrity of the snapshots taken at the
beginning of methods.  I don't think this is necessary, though, so am +1 on
just making the fields volatile.


> You have to do this anyway, unless you sync the
> > entire borrow/return which is a *really* bad idea. Therefore, I think
> > the syncs should be removed from the getters/setters and volatile used
> > instead.
>
> +1
>
>
> Mark
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message