I wrote:
> I'd like to propose a different way to implement ThreadLocal.
> The patch is attached.
I realised that my new version would "leak" the ThreadLocal objects, so
I've attached a new version of the patch that uses a WeakHashMap in
Thread to store the locals.
I liked the previous patch better, but experimentation shows that the
JDK also collects thread locals that aren't referenced anymore.
Regards,
Jeroen
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 12:44:38 -0000
@@ -37,12 +37,8 @@
package java.lang;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
import java.util.WeakHashMap;
+import java.util.Iterator;
/**
* 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)
+ Key key = (Key)keys.next();
+ if (key.get() instanceof InheritableThreadLocal)
{
+ InheritableThreadLocal local =
(InheritableThreadLocal)key.get();
+ 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 WeakHashMap();
+ 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 12:44:38 -0000
@@ -38,6 +38,8 @@
package java.lang;
+import java.util.Map;
+import java.util.WeakHashMap;
/* 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.
+ */
+ WeakHashMap 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 WeakHashMap();
+ }
+ 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 12:44:38 -0000
@@ -37,7 +37,6 @@
package java.lang;
-import java.util.Collections;
import java.util.Map;
import java.util.WeakHashMap;
@@ -98,18 +97,24 @@
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.
+ * Serves as a key for the Thread.locals WeakHashMap.
+ * We can't use "this", because a subclass may override equals/hashCode
+ * and we need to use object identity for the map.
*/
- final Map valueMap = Collections.synchronizedMap(new WeakHashMap());
-
+ final Key key = new Key();
+
+ class Key
+ {
+ Key()
+ {
+ }
+
+ ThreadLocal get()
+ {
+ return ThreadLocal.this;
+ }
+ }
+
/**
* Creates a ThreadLocal object without associating any value to it yet.
*/
@@ -140,14 +145,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(key);
if (value == null)
{
value = initialValue();
- valueMap.put(currentThread, value == null ? NULL : value);
+ map.put(key, value == null ? NULL : value);
}
return value == NULL ? null : value;
}
@@ -162,8 +167,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(key, value == null ? NULL : value);
}
}
_______________________________________________
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
