[cp-patches] RFC: Different ThreadLocal implementation

2005-02-17 Thread Jeroen Frijters
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 -   
1.7
+++ java/lang/InheritableThreadLocal.java   17 Feb 2005 09:36:21 -
@@ -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 -  1.12
+++ java/lang/Thread.java   17 Feb 2005 09:36:22 -
@@ -38,6 +38,8 @@
 
 package java.lang;
 
+import java.util.IdentityHashMap;
+import java.util.Map;
 
 /* Written using Java Class Libraries, 2nd edition, ISBN 

[cp-patches] FYI: IllegalArgumentException in Charset.forName

2005-02-17 Thread Robert Schuster
Hi,
this patch adds support for the officially undocumented behavior of 
throwing an IllegalArgumentException in
java.nio.charset.Charset.forName(String charsetName) if charsetName is null.

A mauve testcase is already provided.
2005-02-18  Robert Schuster [EMAIL PROTECTED]
* java/nio/charset/Charset.java (forName): Throws
IllegalArgumentException when argument is null
and added documentation.

cu
Robert
Index: java/nio/charset/Charset.java
===
RCS file: /cvsroot/classpath/classpath/java/nio/charset/Charset.java,v
retrieving revision 1.14
diff -u -r1.14 Charset.java
--- java/nio/charset/Charset.java	8 Jan 2005 11:48:28 -	1.14
+++ java/nio/charset/Charset.java	18 Feb 2005 01:28:38 -
@@ -127,9 +127,24 @@
   {
 return charsetForName (charsetName) != null;
   }
- 
+
+  /**
+   * Returns the Charset instance for the charset of the given name.
+   * 
+   * @param charsetName
+   * @return
+   * @throws UnsupportedCharsetException if this VM does not support
+   * the charset of the given name.
+   * @throws IllegalCharsetNameException if the given charset name is
+   * legal.
+   * @throws IllegalArgumentException if codecharsetName/code is null.
+   */
   public static Charset forName (String charsetName)
   {
+// Throws IllegalArgumentException as the JDK does.
+if(charsetName == null)
+throw new IllegalArgumentException(Charset name must not be null.);
+
 Charset cs = charsetForName (charsetName);
 if (cs == null)
   throw new UnsupportedCharsetException (charsetName);
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches


Re: fix to LinkedHashMap.java

2005-02-17 Thread Eric Blake
Sorry I haven't cleaned my inbox in a while, you mailed an account I no
longer actively track, since I no longer have time to actively develop on
classpath.  Please send bug reports to the project mailing list, and not
an individual developer, if you want faster response.

According to Jean-Marie White on 11/18/2004 5:44 PM:
 Hi Eric,
 
 just a quick note to let you know that I fixed a bug in LinkedHashMap.java.
 
 In method  void addEntry(Object key, Object value, int idx, boolean
 callRemove), the remove call should read:
 
 remove(root.key)
 
 and not
 
 remove(root)
 
 (You always remove a map entry by key, not by the entry itself)
 
 This may or may not have been fixed, but thought would let you know anyway.
 
 Thanks for your implementing the class in the first place!!!
 jm
 


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath