Shouldn't you synchronize on the pool itself instead of on the trace list in AbandonedObjectPool?
borrowObject and returnObject are synchronized on trace
invalidateObject and removeAbandoned are synchronized on the pool
The synchronization required for using the GenericObjectPool is done when
the respective super methods are called on the object pool.
These syncrhonizations are only for managing abandoned connections, not for the underlying pool. The synchronizations were moved inside the methods to remove the need for a synchronization if removal of abandoned pools is not being used.
You are right, the synchronization for invalidateObject could be moved inside the method like it was done for returnObject.
The entire method for removeAbandoned needs to be synchronized.
Regards,
Glenn
Dirk
[EMAIL PROTECTED] wrote:
glenn 2003/09/23 06:29:30
Modified: dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Log:
When updating to start testing the latest DBCP with bug fixes
I found several performance optimizations in my source tree.
First, use of the Data object was minimized.
Second, use of synchronizations for abandoned traces
were tightened up.
I have been using this in production for quite a while,
just didn't get around to committing it.
Revision Changes Path
1.9 +12 -9 jakarta-commons/dbcp/src/java/org/apache/commons/dbcp/AbandonedObjectPool.java
Index: AbandonedObjectPool.java
===================================================================
RCS file: /home/cvs/jakarta-commons/dbcp/src/java/org/apache/commons/dbcp/AbandonedObjectPool.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- AbandonedObjectPool.java 25 Aug 2003 16:19:59 -0000 1.8
+++ AbandonedObjectPool.java 23 Sep 2003 13:29:30 -0000 1.9
@@ -63,7 +63,6 @@
import java.sql.SQLException;
import java.util.ArrayList;
-import java.util.Date;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
@@ -112,7 +111,7 @@
* * @return Object jdbc Connection
*/
- public synchronized Object borrowObject() throws Exception {
+ public Object borrowObject() throws Exception {
try {
if (config != null
&& config.getRemoveAbandoned()
@@ -125,7 +124,9 @@
((AbandonedTrace)obj).setStackTrace();
}
if (obj != null && config != null && config.getRemoveAbandoned()) {
- trace.add(obj);
+ synchronized(trace) {
+ trace.add(obj);
+ }
}
return obj;
} catch(NoSuchElementException ne) {
@@ -142,9 +143,11 @@
*
* @param Object db Connection to return
*/
- public synchronized void returnObject(Object obj) throws Exception {
+ public void returnObject(Object obj) throws Exception {
if (config != null && config.getRemoveAbandoned()) {
- trace.remove(obj);
+ synchronized(trace) {
+ trace.remove(obj);
+ }
}
super.returnObject(obj);
}
@@ -162,7 +165,7 @@
*/
private synchronized void removeAbandoned() {
// Generate a list of abandoned connections to remove
- long now = new Date().getTime();
+ long now = System.currentTimeMillis();
long timeout = now - (config.getRemoveAbandonedTimeout() * 1000);
ArrayList remove = new ArrayList();
Iterator it = trace.iterator();
1.8 +22 -18 jakarta-commons/dbcp/src/java/org/apache/commons/dbcp/AbandonedTrace.java
Index: AbandonedTrace.java
===================================================================
RCS file: /home/cvs/jakarta-commons/dbcp/src/java/org/apache/commons/dbcp/AbandonedTrace.java,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- AbandonedTrace.java 22 Aug 2003 16:08:31 -0000 1.7
+++ AbandonedTrace.java 23 Sep 2003 13:29:30 -0000 1.8
@@ -90,7 +90,7 @@
private AbandonedTrace parent;
// A stack trace of the code that created me (if in debug mode) **/
private Exception createdBy;
- private Date createdDate;
+ private long createdTime;
// A list of objects created by children of this object
private List trace = new ArrayList();
// Last time this connection was used
@@ -139,7 +139,7 @@
}
if (config.getLogAbandoned()) {
createdBy = new Exception();
- createdDate = new Date();
+ createdTime = System.currentTimeMillis();
}
}
@@ -172,7 +172,7 @@
if (parent != null) {
parent.setLastUsed();
} else {
- lastUsed = new Date().getTime();
+ lastUsed = System.currentTimeMillis();
}
}
@@ -200,7 +200,7 @@
} if (config.getLogAbandoned()) {
createdBy = new Exception();
- createdDate = new Date();
+ createdTime = System.currentTimeMillis();
}
if (parent != null) { parent.addTrace(this);
@@ -213,8 +213,10 @@
*
* @param AbandonedTrace object to add
*/
- protected synchronized void addTrace(AbandonedTrace trace) {
- this.trace.add(trace);
+ protected void addTrace(AbandonedTrace trace) {
+ synchronized(this) {
+ this.trace.add(trace);
+ }
setLastUsed();
}
@@ -223,8 +225,8 @@
* object.
*/
protected synchronized void clearTrace() {
- if (trace != null) {
- trace.clear();
+ if (this.trace != null) {
+ this.trace.clear();
}
}
@@ -241,15 +243,17 @@
* If logAbandoned=true, print a stack trace of the code that
* created this object.
*/
- public synchronized void printStackTrace() {
+ public void printStackTrace() {
if (createdBy != null) {
- System.err.println(format.format(createdDate));
+ System.out.println(format.format(new Date(createdTime)));
createdBy.printStackTrace();
}
- Iterator it = trace.iterator();
- while (it.hasNext()) {
- AbandonedTrace at = (AbandonedTrace)it.next();
- at.printStackTrace();
+ synchronized(this) {
+ Iterator it = this.trace.iterator();
+ while (it.hasNext()) {
+ AbandonedTrace at = (AbandonedTrace)it.next();
+ at.printStackTrace();
+ }
}
}
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
