Hi,
* see code below
I use DBCP, and in our multithreaded performance test of DAOs we found that
multiple threads are basically blocking on the connection pool class, either
waiting to get a connection or to return a connection. In the test most
threads were idle blocking on the DBCP pool, and not doing database work.
This behavior becomes more severe as the number of threads grows.
The problem identified was over synchronization of the pool. Calls to get
and return a connection, were performing database calls, while other threads
block during these calls. This includes resetting connection state (auto
commit), and validation on borrow/return.
I modified the code to avoid this. The changes ensure that all access to
local variables in the pool object are synchronized, but calls to the
database happen on the calling thread. The calls on the caller thread are
the ones that go to the database, and are the expensive ones.
I see a minor problem using assertOpen(), but in a web application
environment the server handles shutdown properly, and we do not add and
remove pools on the fly, so for our needs it is fine. For a general purpose
use, it may not be.
The results are good. The threads spend virtually no time in the connection
pool code after warm-up, and do database work during the test rather than
block. These code changes were reviewed carefully in multithreaded debugger,
and survived our test harness. However, as with any multithreaded code, it
is suspect until proven otherwise :)
I have one grief: It was very difficult to modify this code. I had to copy a
few java files and change small number of lines in each. I would rather
subclass and replace a few methods, or use IOC to inject modified classes
rather than change hard coding of actual implementation classes in many
files. I think a spring like approach will make the code more developer
friendly and encourage more contributions.
Any feedback is welcome.
--------------------------------
public Object borrowObject() throws Exception {
assertOpen();
long starttime = System.currentTimeMillis();
for (;;) {
ObjectTimestampPair pair = null;
// EA: Added synchronization on shared variable and
notification
synchronized (this) {
// if there are any sleeping, just grab one
of those
try {
pair = (ObjectTimestampPair)
(_pool.removeFirst());
} catch (NoSuchElementException e) {
; /* ignored */
}
// otherwise
if (null == pair) {
// check if we can create one
// (note we know that the num
sleeping is 0, else we wouldn't be here)
if (_maxActive < 0 || _numActive <
_maxActive) {
// allow new object to be
created
} else {
// the pool is exhausted
switch
(_whenExhaustedAction) {
case WHEN_EXHAUSTED_GROW:
// allow new object
to be created
break;
case WHEN_EXHAUSTED_FAIL:
throw new
NoSuchElementException("Pool exhausted");
case WHEN_EXHAUSTED_BLOCK:
try {
if (_maxWait
<= 0) {
wait();
} else {
//
this code may be executed again after a notify then
//
continue cycle
//
so, need to calculate the amount of time to wait
final long elapsed = (System.currentTimeMillis() - starttime);
final long waitTime = _maxWait - elapsed;
if
(waitTime > 0) {
wait(waitTime);
}
}
} catch
(InterruptedException e) {
// ignored
}
if (_maxWait > 0
&&
((System.currentTimeMillis() - starttime) >= _maxWait)) {
throw new
NoSuchElementException(
"Timeout waiting for idle object");
} else {
continue; //
keep looping
}
default:
throw new
IllegalArgumentException(
"WhenExhaustedAction property " + _whenExhaustedAction
+ " not recognized.");
}
}
}
_numActive++;
}
// create new object when needed
boolean newlyCreated = false;
if (null == pair) {
try {
Object obj = _factory.makeObject();
pair = new ObjectTimestampPair(obj);
newlyCreated = true;
} finally {
if (!newlyCreated) {
// EA: Added synchronization
on shared variable and notification
synchronized (this) {
// object cannot be
created
_numActive--;
notifyAll();
}
}
}
}
// activate & validate the object
try {
_factory.activateObject(pair.value);
if (_testOnBorrow &&
!_factory.validateObject(pair.value)) {
throw new Exception("ValidateObject
failed");
}
return pair.value;
} catch (Throwable e) {
// object cannot be activated or is invalid
// EA added synchronization on access to
shared variables and
// notification:
synchronized (this) {
// object cannot be activated or is
invalid
_numActive--;
notifyAll();
}
try {
_factory.destroyObject(pair.value);
} catch (Throwable e2) {
// cannot destroy broken object
}
if (newlyCreated) {
throw new NoSuchElementException(
"Could not create a
validated object, cause: " + e.getMessage());
} else {
continue; // keep looping
}
}
}
}
// EA Removed synchronization
public void returnObject(Object obj) throws Exception {
assertOpen();
addObjectToPool(obj, true);
}
private void addObjectToPool(Object obj, boolean decrementNumActive)
throws Exception {
boolean success = true;
if (_testOnReturn && !(_factory.validateObject(obj))) {
success = false;
} else {
try {
_factory.passivateObject(obj);
} catch (Exception e) {
success = false;
}
}
boolean shouldDestroy = !success;
// EA Added synchronization on shared access variables and
notification
synchronized (this) {
if (decrementNumActive) {
_numActive--;
}
// EA Added local variable with synchronized access,
to be on the safe
// side
final int _maxIdle = getMaxIdle();
if ((_maxIdle >= 0) && (_pool.size() >= _maxIdle)) {
shouldDestroy = true;
} else if (success) {
_pool.addLast(new ObjectTimestampPair(obj));
}
notifyAll(); // _numActive has changed
}
if (shouldDestroy) {
try {
_factory.destroyObject(obj);
} catch (Exception e) {
// ignored
}
}
}
/**
* EA: Removed outside synchronization, synchronized shared variables and
* notifications inside.
*/
public void invalidateObject(Object obj) throws Exception {
assertOpen();
try {
_factory.destroyObject(obj);
} finally {
// EA Added synchronization on shared access
variables and notification
synchronized (this) {
_numActive--;
notifyAll(); // _numActive has changed
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
