Hi java.util.logging maintainers,
(Will this get Serguei or Graham to come out of retirement?! Maybe not.)
Description:
The removal of unnecessary optimization below
(replacing synchronized with j.u.c. or volatile replacements)
made a big difference in one of our local applications with
many concurrently logging threads.
As well, making levelObject volatile slightly improves the thread-safety
of Logger.
Fix written by Jeremy Manson and reviewed by Bill Pugh
(both of JSR-133 fame), and by myself.
There are further concurrency improvements that can be made to this code,
but we are not that ambitious.
# HG changeset patch
# User martin
# Date 1232071338 28800
# Node ID 7ac6b44ba1709d554282cc16b5b8b2c613bc1079
# Parent 7f6969c090750e1d389a93c3a657b60426d3d593
6666666: Remove synchronization bottlenecks in Logger
Reviewed-by: xxxxx
Contributed-by: jeremyman...@google.com
diff --git a/src/share/classes/java/util/logging/Logger.java
b/src/share/classes/java/util/logging/Logger.java
--- a/src/share/classes/java/util/logging/Logger.java
+++ b/src/share/classes/java/util/logging/Logger.java
@@ -27,6 +27,7 @@
package java.util.logging;
import java.util.*;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.security.*;
import java.lang.ref.WeakReference;
@@ -165,10 +166,11 @@
private static final int offValue = Level.OFF.intValue();
private LogManager manager;
private String name;
- private ArrayList<Handler> handlers;
+ private final CopyOnWriteArrayList<Handler> handlers =
+ new CopyOnWriteArrayList<Handler>();
private String resourceBundleName;
- private boolean useParentHandlers = true;
- private Filter filter;
+ private volatile boolean useParentHandlers = true;
+ private volatile Filter filter;
private boolean anonymous;
private ResourceBundle catalog; // Cached resource bundle
@@ -180,9 +182,9 @@
private static Object treeLock = new Object();
// We keep weak references from parents to children, but strong
// references from children to parents.
- private Logger parent; // our nearest parent.
+ private volatile Logger parent; // our nearest parent.
private ArrayList<WeakReference<Logger>> kids; //
WeakReferences to loggers that have us as parent
- private Level levelObject;
+ private volatile Level levelObject;
private volatile int levelValue; // current effective level value
/**
@@ -438,7 +440,7 @@
* @exception SecurityException if a security manager exists and if
* the caller does not have LoggingPermission("control").
*/
- public synchronized void setFilter(Filter newFilter) throws
SecurityException {
+ public void setFilter(Filter newFilter) throws SecurityException {
checkAccess();
filter = newFilter;
}
@@ -448,7 +450,7 @@
*
* @return a filter object (may be null)
*/
- public synchronized Filter getFilter() {
+ public Filter getFilter() {
return filter;
}
@@ -465,10 +467,9 @@
if (record.getLevel().intValue() < levelValue || levelValue
== offValue) {
return;
}
- synchronized (this) {
- if (filter != null && !filter.isLoggable(record)) {
- return;
- }
+ Filter theFilter = filter;
+ if (theFilter != null && !theFilter.isLoggable(record)) {
+ return;
}
// Post the LogRecord to all our Handlers, and then to
@@ -1182,13 +1183,10 @@
* @exception SecurityException if a security manager exists and if
* the caller does not have LoggingPermission("control").
*/
- public synchronized void addHandler(Handler handler) throws
SecurityException {
+ public void addHandler(Handler handler) throws SecurityException {
// Check for null handler
handler.getClass();
checkAccess();
- if (handlers == null) {
- handlers = new ArrayList<Handler>();
- }
handlers.add(handler);
}
@@ -1201,12 +1199,9 @@
* @exception SecurityException if a security manager exists and if
* the caller does not have LoggingPermission("control").
*/
- public synchronized void removeHandler(Handler handler) throws
SecurityException {
+ public void removeHandler(Handler handler) throws SecurityException {
checkAccess();
if (handler == null) {
- return;
- }
- if (handlers == null) {
return;
}
handlers.remove(handler);
@@ -1217,11 +1212,8 @@
* <p>
* @return an array of all registered Handlers
*/
- public synchronized Handler[] getHandlers() {
- if (handlers == null) {
- return emptyHandlers;
- }
- return handlers.toArray(new Handler[handlers.size()]);
+ public Handler[] getHandlers() {
+ return handlers.toArray(emptyHandlers);
}
/**
@@ -1235,7 +1227,7 @@
* @exception SecurityException if a security manager exists and if
* the caller does not have LoggingPermission("control").
*/
- public synchronized void setUseParentHandlers(boolean useParentHandlers) {
+ public void setUseParentHandlers(boolean useParentHandlers) {
checkAccess();
this.useParentHandlers = useParentHandlers;
}
@@ -1246,7 +1238,7 @@
*
* @return true if output is to be sent to the logger's parent
*/
- public synchronized boolean getUseParentHandlers() {
+ public boolean getUseParentHandlers() {
return useParentHandlers;
}
@@ -1354,9 +1346,12 @@
* @return nearest existing parent Logger
*/
public Logger getParent() {
- synchronized (treeLock) {
- return parent;
- }
+ // Note: this used to be synchronized on treeLock. However, this only
+ // provided memory semantics, as there was no guarantee that the caller
+ // would synchronize on treeLock (in fact, there is no way for external
+ // callers to so synchronize. Therefore, we have made parent volatile
+ // instead.
+ return parent;
}
/**
