On Tue, Dec 21, 2010 at 6:01 PM, sebb <seb...@gmail.com> wrote: > On 21 December 2010 22:44, Mark Thomas <ma...@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-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
