Author: markt
Date: Tue Jun 12 12:38:20 2012
New Revision: 1349298
URL: http://svn.apache.org/viewvc?rev=1349298&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52999
Remove synchronization bottleneck in container event handling
Modified:
tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java
Modified: tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java?rev=1349298&r1=1349297&r2=1349298&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java Tue Jun 12
12:38:20 2012
@@ -180,7 +180,9 @@ public abstract class ContainerBase exte
/**
* The container event listeners for this Container.
*/
- protected ArrayList<ContainerListener> listeners = new
ArrayList<ContainerListener>();
+ protected ArrayList<ContainerListener> listeners =
+ new ArrayList<ContainerListener>();
+ protected ReadWriteLock listenersLock = new ReentrantReadWriteLock();
/**
@@ -896,10 +898,13 @@ public abstract class ContainerBase exte
@Override
public void addContainerListener(ContainerListener listener) {
- synchronized (listeners) {
+ Lock write = listenersLock.writeLock();
+ write.lock();
+ try {
listeners.add(listener);
+ } finally {
+ write.unlock();
}
-
}
@@ -957,12 +962,15 @@ public abstract class ContainerBase exte
@Override
public ContainerListener[] findContainerListeners() {
- synchronized (listeners) {
+ Lock read = listenersLock.readLock();
+ read.lock();
+ try {
ContainerListener[] results =
new ContainerListener[listeners.size()];
return listeners.toArray(results);
+ } finally {
+ read.unlock();
}
-
}
@@ -1017,10 +1025,13 @@ public abstract class ContainerBase exte
@Override
public void removeContainerListener(ContainerListener listener) {
- synchronized (listeners) {
+ Lock write = listenersLock.writeLock();
+ write.lock();
+ try {
listeners.remove(listener);
+ } finally {
+ write.unlock();
}
-
}
@@ -1365,16 +1376,31 @@ public abstract class ContainerBase exte
@Override
public void fireContainerEvent(String type, Object data) {
- if (listeners.size() < 1)
- return;
- ContainerEvent event = new ContainerEvent(this, type, data);
- ContainerListener list[] = new ContainerListener[0];
- synchronized (listeners) {
- list = listeners.toArray(list);
+ /*
+ * Implementation note
+ * There are two options here.
+ * 1) Take a copy of listeners and fire the events outside of the read
+ * lock
+ * 2) Don't take a copy and fire the events inside the read lock
+ *
+ * Approach 2 has been used here since holding the read lock only
+ * prevents writes and that is preferable to creating lots of array
+ * objects. Since writes occur on start / stop (unless an external
+ * management tool is used) then holding the read lock for a relatively
+ * long time should not be an issue.
+ */
+ Lock read = listenersLock.readLock();
+ read.lock();
+ try {
+ if (listeners.size() < 1)
+ return;
+ ContainerEvent event = new ContainerEvent(this, type, data);
+ for (ContainerListener listener : listeners) {
+ listener.containerEvent(event);
+ }
+ } finally {
+ read.unlock();
}
- for (int i = 0; i < list.length; i++)
- list[i].containerEvent(event);
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
