Hi, I'd like to propose a different way to implement ThreadLocal. The patch is attached. It trades using WeakHashMaps for one additional field in Thread, which I think is a good trade. It is also completely lock free.
Note that it also fixes a bug in InheritableThreadLocal: - local.valueMap.put(childThread, (childValue == null - ? NULL : parentValue)); Any comments? Regards, Jeroen 2005-02-17 Jeroen Frijters <[EMAIL PROTECTED]> * java/lang/InheritableThreadLocal.java (threadMap): Removed. (InheritableThreadLocal): Removed code. (newChildThread): Changed to use locals map in Thread. * java/lang/Thread.java (locals): New field. (die): Clear locals field. (getThreadLocals): New method. * java/lang/ThreadLocal.java (value): Removed. (valueMap): Removed. (get,set): Changed to use locals map in Thread.
Index: java/lang/InheritableThreadLocal.java =================================================================== RCS file: /cvsroot/classpath/classpath/java/lang/InheritableThreadLocal.java,v retrieving revision 1.7 diff -u -r1.7 InheritableThreadLocal.java --- java/lang/InheritableThreadLocal.java 9 Aug 2003 18:47:04 -0000 1.7 +++ java/lang/InheritableThreadLocal.java 17 Feb 2005 09:36:21 -0000 @@ -37,12 +37,8 @@ package java.lang; -import java.util.ArrayList; -import java.util.Collections; +import java.util.IdentityHashMap; import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.WeakHashMap; /** * A ThreadLocal whose value is inherited by child Threads. The value of the @@ -64,30 +60,11 @@ public class InheritableThreadLocal extends ThreadLocal { /** - * Maps Threads to a List of InheritableThreadLocals (the heritage of that - * Thread). Uses a WeakHashMap so if the Thread is garbage collected the - * List can be collected, too. Maps to a list in case the user overrides - * equals. - */ - private static final Map threadMap - = Collections.synchronizedMap(new WeakHashMap()); - - /** * Creates a new InheritableThreadLocal that has no values associated * with it yet. */ public InheritableThreadLocal() { - Thread currentThread = Thread.currentThread(); - // Note that we don't have to synchronize, as only this thread will - // ever modify the returned heritage and threadMap is a synchronizedMap. - List heritage = (List) threadMap.get(currentThread); - if (heritage == null) - { - heritage = new ArrayList(); - threadMap.put(currentThread, heritage); - } - heritage.add(this); } /** @@ -116,25 +93,22 @@ { // The currentThread is the parent of the new thread. Thread parentThread = Thread.currentThread(); - // Note that we don't have to synchronize, as only this thread will - // ever modify the returned heritage and threadMap is a synchronizedMap. - ArrayList heritage = (ArrayList) threadMap.get(parentThread); - if (heritage != null) + if (parentThread.locals != null) { - threadMap.put(childThread, heritage.clone()); - // Perform the inheritance. - Iterator it = heritage.iterator(); - int i = heritage.size(); - while (--i >= 0) + Iterator keys = parentThread.locals.keySet().iterator(); + while (keys.hasNext()) { - InheritableThreadLocal local = (InheritableThreadLocal) it.next(); - Object parentValue = local.valueMap.get(parentThread); - if (parentValue != null) + Object key = keys.next(); + if (key instanceof InheritableThreadLocal) { + InheritableThreadLocal local = (InheritableThreadLocal)key; + Object parentValue = parentThread.locals.get(key); Object childValue = local.childValue(parentValue == NULL ? null : parentValue); - local.valueMap.put(childThread, (childValue == null - ? NULL : parentValue)); + if (childThread.locals == null) + childThread.locals = new IdentityHashMap(); + childThread.locals.put(key, (childValue == null + ? NULL : childValue)); } } } Index: java/lang/Thread.java =================================================================== RCS file: /cvsroot/classpath/classpath/java/lang/Thread.java,v retrieving revision 1.12 diff -u -r1.12 Thread.java --- java/lang/Thread.java 31 Dec 2004 21:32:17 -0000 1.12 +++ java/lang/Thread.java 17 Feb 2005 09:36:22 -0000 @@ -38,6 +38,8 @@ package java.lang; +import java.util.IdentityHashMap; +import java.util.Map; /* Written using "Java Class Libraries", 2nd edition, ISBN 0-201-31002-3 * "The Java Language Specification", ISBN 0-201-63451-1 @@ -131,6 +133,11 @@ /** The next thread number to use. */ private static int numAnonymousThreadsCreated; + /** Thread local storage. Package accessible for use by + * InheritableThreadLocal. + */ + Map locals; + /** * Allocates a new <code>Thread</code> object. This constructor has * the same effect as <code>Thread(null, null,</code> @@ -973,5 +980,20 @@ { group.removeThread(this); vmThread = null; + locals = null; + } + + /** + * Returns the map used by ThreadLocal to store the thread local values. + */ + static Map getThreadLocals() + { + Thread thread = currentThread(); + Map locals = thread.locals; + if (locals == null) + { + locals = thread.locals = new IdentityHashMap(); + } + return locals; } } Index: java/lang/ThreadLocal.java =================================================================== RCS file: /cvsroot/classpath/classpath/java/lang/ThreadLocal.java,v retrieving revision 1.5 diff -u -r1.5 ThreadLocal.java --- java/lang/ThreadLocal.java 15 Oct 2004 07:44:17 -0000 1.5 +++ java/lang/ThreadLocal.java 17 Feb 2005 09:36:22 -0000 @@ -98,19 +98,6 @@ static final Object NULL = new Object(); /** - * The stored value. Package visible for use by InheritableThreadLocal. */ - Object value; - - /** - * Maps Threads to values. Uses a WeakHashMap so if a Thread is garbage - * collected the reference to the Value will disappear. A null value means - * uninitialized, while NULL means a user-specified null. Only the - * <code>set(Thread, Object)</code> and <code>get(Thread)</code> methods - * access it. Package visible for use by InheritableThreadLocal. - */ - final Map valueMap = Collections.synchronizedMap(new WeakHashMap()); - - /** * Creates a ThreadLocal object without associating any value to it yet. */ public ThreadLocal() @@ -140,14 +127,14 @@ */ public Object get() { - Thread currentThread = Thread.currentThread(); + Map map = Thread.getThreadLocals(); // Note that we don't have to synchronize, as only this thread will - // ever modify the returned value and valueMap is a synchronizedMap. - Object value = valueMap.get(currentThread); + // ever modify the map. + Object value = map.get(this); if (value == null) { value = initialValue(); - valueMap.put(currentThread, value == null ? NULL : value); + map.put(this, value == null ? NULL : value); } return value == NULL ? null : value; } @@ -162,8 +149,9 @@ */ public void set(Object value) { + Map map = Thread.getThreadLocals(); // Note that we don't have to synchronize, as only this thread will - // ever modify the returned value and valueMap is a synchronizedMap. - valueMap.put(Thread.currentThread(), value == null ? NULL : value); + // ever modify the map. + map.put(this, value == null ? NULL : value); } }
_______________________________________________ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches