Re: Another 646 patch and performance comparison with TOWER

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Martin" == Martin Kahlert <martin.kahlert@infineon.com> writes:

Martin> I would like to see this patch in 3.1.
Martin> It replaces the infamous Solaris encoding "646"
Martin> by "8859_1" in DEFAULT_FILE_ENCODING.

I'd like to understand the problem further.

Martin> Using the new default speeds up my big pure Java application
Martin> from 47.7 seconds to 1.7 seconds (without any optimization)
Martin> and 1.37 seconds with -O2.  This makes the runtime comparable
Martin> to the runtime obtained with the TOWER compiler (1.47
Martin> seconds).

Ok.  We definitely need to do something.  A 40x slowdown is
unacceptable.

Martin> The unpatched version tried to open a lot of non-existing libs
Martin> causing the immense slowdown

This is interesting.  It suggests that it would make sense for us to
cache the name->encoding-class lookup.  That ought to reduce the
number of bad lookups to just one (or one set) per run.  This will
help all platforms and encodings, not just 646 on Solaris.

The downside here is that then we can't GC encoder classes, or swap
them out at runtime.  I don't think this is a big loss.

Could you try this patch?  Be sure, of course, to back out your
current patch before applying this one.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	* gnu/gcj/convert/BytesToUnicode.java (nameMap): New static field.
	(getDecoder): Now synchronized.  Use nameMap.
	* gnu/gcj/convert/UnicodeToBytes.java (nameMap): New static field.
	(getEncoder): Now synchronized.  Use nameMap.

Index: gnu/gcj/convert/BytesToUnicode.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/gcj/convert/BytesToUnicode.java,v
retrieving revision 1.9
diff -u -r1.9 BytesToUnicode.java
--- gnu/gcj/convert/BytesToUnicode.java 2001/07/30 20:24:17 1.9
+++ gnu/gcj/convert/BytesToUnicode.java 2002/03/29 20:16:48
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999, 2000, 2001  Free Software Foundation
+/* Copyright (C) 1999, 2000, 2001, 2002  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -8,6 +8,8 @@
 
 package gnu.gcj.convert;
 
+import java.util.HashMap;
+
 public abstract class BytesToUnicode extends IOConverter
 {
   /** Buffer to read bytes from.
@@ -33,6 +35,11 @@
     = new BytesToUnicode[CACHE_SIZE];
   private static int currCachePos = 0;
 
+  // We also keep track of which class implements a given encoder
+  // name.  This lets us avoid lots of runtime lookups when the cache
+  // of reusable encoders is empty.
+  private static HashMap nameMap = new HashMap ();
+
   public abstract String getName();
 
   public static BytesToUnicode getDefaultDecoder()
@@ -70,48 +77,71 @@
   }
 
   /** Get a byte-stream->char-stream converter given an encoding name. */
-  public static BytesToUnicode getDecoder (String encoding)
+  public static synchronized BytesToUnicode getDecoder (String encoding)
     throws java.io.UnsupportedEncodingException
   {
     /* First hunt in our cache to see if we have a decoder that is
        already allocated. */
-    synchronized (BytesToUnicode.class)
+    int i;
+    for (i = 0; i < decoderCache.length; ++i)
       {
-	int i;
-	for (i = 0; i < decoderCache.length; ++i)
+	if (decoderCache[i] != null
+	    && encoding.equals(decoderCache[i].getName ()))
 	  {
-	    if (decoderCache[i] != null
-		&& encoding.equals(decoderCache[i].getName ()))
-	      {
-		BytesToUnicode rv = decoderCache[i];
-		decoderCache[i] = null;
-		return rv;
-	    }
+	    BytesToUnicode rv = decoderCache[i];
+	    decoderCache[i] = null;
+	    return rv;
 	  }
       }
 
     // It's not in the cache, so now we have to do real work.
     String className = "gnu.gcj.convert.Input_" + canonicalize (encoding);
-    Class decodingClass;
-    try 
-      { 
-	decodingClass = Class.forName(className); 
-	return (BytesToUnicode) decodingClass.newInstance();
-      } 
-    catch (Throwable ex) 
-      { 
+    BytesToUnicode result = null;
+
+    Class decodingClass = (Class) nameMap.get (encoding);
+    if (decodingClass == null)
+      {
+	try
+	  {
+	    decodingClass = Class.forName(className);
+	    return (BytesToUnicode) decodingClass.newInstance();
+	  }
+	catch (Throwable ex)
+	  {
+	    // We didn't find the class, or construction failed.  Try
+	    // iconv.
+	    decodingClass = Input_iconv.class;
+	  }
+      }
+
+    if (result == null)
+      {
+	// Try again using the new class.
 	try
 	  {
-	    // We pass the original name to iconv and let it handle
-	    // its own aliasing.
-	    return new Input_iconv (encoding);
+	    if (decodingClass == Input_iconv.class)
+	      {
+		// We pass the original name to iconv and let it handle
+		// its own aliasing.
+		result = new Input_iconv (encoding);
+	      }
+	    else
+	      {
+		// This case occurs when we saw the name in the cache.
+		result = (BytesToUnicode) decodingClass.newInstance ();
+	      }
 	  }
-	catch (Throwable _)
+	catch (Throwable ex)
 	  {
 	    throw new java.io.UnsupportedEncodingException(encoding
 							   + " (" + ex + ')');
 	  }
       }
+
+    // At this point we have a result.  Let's cache the class we used,
+    // so we don't have to do expensive runtime searching the next time.
+    nameMap.put (encoding, decodingClass);
+    return result;
   }
 
   /** Make input bytes available to the conversion.
Index: gnu/gcj/convert/UnicodeToBytes.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/gcj/convert/UnicodeToBytes.java,v
retrieving revision 1.9
diff -u -r1.9 UnicodeToBytes.java
--- gnu/gcj/convert/UnicodeToBytes.java 2001/08/18 03:56:01 1.9
+++ gnu/gcj/convert/UnicodeToBytes.java 2002/03/29 20:16:48
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999, 2000, 2001  Free Software Foundation
+/* Copyright (C) 1999, 2000, 2001, 2002  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -8,6 +8,8 @@
 
 package gnu.gcj.convert; 
 
+import java.util.HashMap;
+
 public abstract class UnicodeToBytes extends IOConverter
 {
   /** Buffer to emit bytes to.
@@ -30,6 +32,11 @@
     = new UnicodeToBytes[CACHE_SIZE];
   private static int currCachePos = 0;
 
+  // We also keep track of which class implements a given encoder
+  // name.  This lets us avoid lots of runtime lookups when the cache
+  // of reusable encoders is empty.
+  private static HashMap nameMap = new HashMap ();
+
   public abstract String getName();
 
   public static UnicodeToBytes getDefaultEncoder()
@@ -68,48 +75,73 @@
   }
 
   /** Get a char-stream->byte-stream converter given an encoding name. */
-  public static UnicodeToBytes getEncoder (String encoding)
+  public static synchronized UnicodeToBytes getEncoder (String encoding)
     throws java.io.UnsupportedEncodingException
   {
     /* First hunt in our cache to see if we have a encoder that is
        already allocated. */
-    synchronized (UnicodeToBytes.class)
+    int i;
+    for (i = 0; i < encoderCache.length; ++i)
       {
-	int i;
-	for (i = 0; i < encoderCache.length; ++i)
+	if (encoderCache[i] != null
+	    && encoding.equals(encoderCache[i].getName ()))
 	  {
-	    if (encoderCache[i] != null
-		&& encoding.equals(encoderCache[i].getName ()))
-	      {
-		UnicodeToBytes rv = encoderCache[i];
-		encoderCache[i] = null;
-		return rv;
-	    }
+	    UnicodeToBytes rv = encoderCache[i];
+	    encoderCache[i] = null;
+	    return rv;
 	  }
       }
 
     String className = "gnu.gcj.convert.Output_" + canonicalize (encoding);
-    Class encodingClass;
-    try 
-      { 
-	encodingClass = Class.forName(className); 
-	return (UnicodeToBytes) encodingClass.newInstance();
-      } 
-    catch (Throwable ex) 
-      { 
+    UnicodeToBytes result = null;
+
+    Class encodingClass = (Class) nameMap.get (encoding);
+    if (encodingClass == null)
+      {
+	try
+	  {
+	    // Try to find the class.
+	    encodingClass = Class.forName(className); 
+	    result = (UnicodeToBytes) encodingClass.newInstance();
+	  }
+	catch (Throwable ex)
+	  {
+	    // We didn't find the class, or construction failed.  Try
+	    // iconv.
+	    encodingClass = Output_iconv.class;
+	  }
+      }
+
+    if (result == null)
+      {
+	// Try again using the new class.
 	try
 	  {
-	    // We pass the original name to iconv and let it handle
-	    // its own aliasing.
-	    return new Output_iconv (encoding);
+	    if (encodingClass == Output_iconv.class)
+	      {
+		// We pass the original name to iconv and let it handle
+		// its own aliasing.  We also circumvent the name cache
+		// system here.
+		result = new Output_iconv (encoding);
+	      }
+	    else
+	      {
+		// This case occurs when we saw the name in the cache.
+		result = (UnicodeToBytes) encodingClass.newInstance ();
+	      }
 	  }
-	catch (Throwable _)
+	catch (Throwable ex)
 	  {
-	    // Put the original exception in the throwable.
+	    // Put the exception in the throwable.
 	    throw new java.io.UnsupportedEncodingException(encoding + " ("
 							   + ex + ')');
 	  }
       }
+
+    // At this point we have a result.  Let's cache the class we used,
+    // so we don't have to do expensive runtime searching the next time.
+    nameMap.put (encoding, encodingClass);
+    return result;
   }
 
   public final void setOutput(byte[] buffer, int count)