You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by gl...@apache.org on 2003/09/23 15:29:30 UTC
cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp AbandonedObjectPool.java AbandonedTrace.java
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Good catch. syncrhonization changes committed to CVS.
Glenn
Dirk Verbeeck wrote:
> I was thinking about concurrent modification of the trace List
>
> thread1: borrowObject -> removeAbandoned (synchronized on pool) ->
> trace.iterator()
> thread2: returnObject -> (synchronize on trace) -> trace.remove
>
> This triggers a ConcurrentModificationException, IMHO
>
> I would synchronize on the pool itself like this:
> synchronized(this) {
> trace.remove(obj);
> }
>
> Dirk
>
> Glenn Nielsen wrote:
>
>> Dirk Verbeeck wrote:
>>
>>> 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
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Good catch. syncrhonization changes committed to CVS.
Glenn
Dirk Verbeeck wrote:
> I was thinking about concurrent modification of the trace List
>
> thread1: borrowObject -> removeAbandoned (synchronized on pool) ->
> trace.iterator()
> thread2: returnObject -> (synchronize on trace) -> trace.remove
>
> This triggers a ConcurrentModificationException, IMHO
>
> I would synchronize on the pool itself like this:
> synchronized(this) {
> trace.remove(obj);
> }
>
> Dirk
>
> Glenn Nielsen wrote:
>
>> Dirk Verbeeck wrote:
>>
>>> 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
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Dirk Verbeeck <di...@pandora.be>.
I was thinking about concurrent modification of the trace List
thread1: borrowObject -> removeAbandoned (synchronized on pool) ->
trace.iterator()
thread2: returnObject -> (synchronize on trace) -> trace.remove
This triggers a ConcurrentModificationException, IMHO
I would synchronize on the pool itself like this:
synchronized(this) {
trace.remove(obj);
}
Dirk
Glenn Nielsen wrote:
> Dirk Verbeeck wrote:
>
>> 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
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Dirk Verbeeck <di...@pandora.be>.
I was thinking about concurrent modification of the trace List
thread1: borrowObject -> removeAbandoned (synchronized on pool) ->
trace.iterator()
thread2: returnObject -> (synchronize on trace) -> trace.remove
This triggers a ConcurrentModificationException, IMHO
I would synchronize on the pool itself like this:
synchronized(this) {
trace.remove(obj);
}
Dirk
Glenn Nielsen wrote:
> Dirk Verbeeck wrote:
>
>> 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
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Dirk Verbeeck wrote:
> 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
>
>
> glenn@apache.org 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: commons-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>>
>>
>>
>>
>>
>>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Dirk Verbeeck wrote:
> 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
>
>
> glenn@apache.org 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: commons-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>>
>>
>>
>>
>>
>>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Dirk Verbeeck <di...@pandora.be>.
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
Dirk
glenn@apache.org 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: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp
AbandonedObjectPool.java AbandonedTrace.java
Posted by Dirk Verbeeck <di...@pandora.be>.
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
Dirk
glenn@apache.org 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: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>
>
>
>