Re: [cp-patches] Patch: use StringBuilder for reading/writing property files

[Mark Wielaard]

Hi Anthony,

On Fri, 2005-10-28 at 17:05 -0700, Anthony Green wrote:
> On Fri, 2005-10-28 at 15:40 +0200, Mark Wielaard wrote:
> > Interesting. What was the total running time before/after this patch?
> 
> I don't have the modified library anymore.
> 
> > Could you post your test program?
> 
> import java.io.*;
> import java.util.*;
> 
> public class pread
> {
>     public static void main(String args[]) throws Exception
>     {
>       Properties p = new Properties();
>       int i = 0;
>       while (i != args.length)
>         p.load (new FileInputStream (args[i++]));
>     }
> }

Interesting testcase. Running it with jamvm on all the property files in
GNU Classpath CVS gives a running time of 15.01 seconds (best of 3).
Since we don't have an unsynchronized StringBuffer in classpath yet I
looked at the source and added a quick hack in case the line we are
reading doesn't contain any escaped chars. In such a case we don't need
to really iterate through everything and append it, but we can just get
the substrings and be done with it. That, plus your original patch using
StringBuilder, brings down the running time to 13.86 seconds (best of 3
again). Patch attached, no mauve regressions (there are 5 failures in
AcuniaPropertiesTest before and after the patch that I didn't
investigate yet).

Another thing to try and optimize here is to just do a line.getChars()
and iterate over the char[] instead of doing a line.charAt() for each
position. Opinions? Volunteers? Anyone want to test against other
runtimes? Or another test set?

Cheers,

Mark

Index: java/util/Properties.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/Properties.java,v
retrieving revision 1.34
diff -u -r1.34 Properties.java
--- java/util/Properties.java   23 Oct 2005 21:56:31 -0000      1.34
+++ java/util/Properties.java   30 Oct 2005 11:09:05 -0000
@@ -219,12 +219,15 @@
 
         // The characters up to the next Whitespace, ':', or '='
         // describe the key.  But look for escape sequences.
-        StringBuffer key = new StringBuffer();
+       // Try to short-circuit when there is no escape char.
+       int start = pos;
+       boolean needsEscape = line.indexOf('\\', pos) != -1;
+        StringBuilder key = needsEscape ? new StringBuilder() : null;
         while (pos < line.length()
                && ! Character.isWhitespace(c = line.charAt(pos++))
                && c != '=' && c != ':')
           {
-            if (c == '\\')
+            if (needsEscape && c == '\\')
               {
                 if (pos == line.length())
                   {
@@ -268,11 +271,20 @@
                       }
                   }
               }
-            else
+            else if (needsEscape)
               key.append(c);
           }
 
         boolean isDelim = (c == ':' || c == '=');
+
+       String keyString;
+       if (needsEscape)
+         keyString = key.toString();
+       else if (isDelim || Character.isWhitespace(c))
+         keyString = line.substring(start, pos - 1);
+       else
+         keyString = line.substring(start, pos);
+
         while (pos < line.length()
                && Character.isWhitespace(c = line.charAt(pos)))
           pos++;
@@ -285,7 +297,15 @@
               pos++;
           }
 
-        StringBuffer element = new StringBuffer(line.length() - pos);
+       // Short-circuit if no escape chars found.
+       if (!needsEscape)
+         {
+           put(keyString, line.substring(pos));
+           continue;
+         }
+
+       // Escape char found so iterate through the rest of the line.
+        StringBuilder element = new StringBuilder(line.length() - pos);
         while (pos < line.length())
           {
             c = line.charAt(pos++);
@@ -341,7 +361,7 @@
             else
               element.append(c);
           }
-        put(key.toString(), element.toString());
+        put(keyString, element.toString());
       }
   }
 
@@ -405,7 +425,7 @@
     
     Iterator iter = entrySet ().iterator ();
     int i = size ();
-    StringBuffer s = new StringBuffer (); // Reuse the same buffer.
+    StringBuilder s = new StringBuilder (); // Reuse the same buffer.
     while (--i >= 0)
       {
         Map.Entry entry = (Map.Entry) iter.next ();
@@ -548,7 +568,7 @@
    *        leading spaces must be escaped for the value
    * @see #store(OutputStream, String)
    */
-  private void formatForOutput(String str, StringBuffer buffer, boolean key)
+  private void formatForOutput(String str, StringBuilder buffer, boolean key)
   {
     if (key)
       {

signature.asc
Description: This is a digitally signed message part