Optimize Formatter.parse (including String.printf)

[Martin Buchholz]

On Wed, Nov 4, 2009 at 23:07, Xueming Shen <Xueming.Shen at sun.com> wrote:
> #2659:
>
> conversion(m.group(idx++));
>
> ->  conversion(m.group(idx)); ?

Done.

I couldn't help making this additonal change:

diff --git a/src/share/classes/java/util/Formatter.java
b/src/share/classes/java/util/Formatter.java
--- a/src/share/classes/java/util/Formatter.java
+++ b/src/share/classes/java/util/Formatter.java
@@ -2503,7 +2503,7 @@
                     al.add(new FixedString(s.substring(i, m.start())));
                 }

-                al.add(new FormatSpecifier(this, m));
+                al.add(new FormatSpecifier(m));
                 i = m.end();
             } else {
                 // No more valid format specifiers.  Check for possible invalid
@@ -2552,7 +2552,7 @@
         private boolean dt = false;
         private char c;

-        private Formatter formatter;
+        //private Formatter formatter;

         // cache the line separator
         private String ls;
@@ -2640,8 +2640,7 @@
             return c;
         }

-        FormatSpecifier(Formatter formatter, Matcher m) {
-            this.formatter = formatter;
+        FormatSpecifier(Matcher m) {
             int idx = 1;

             index(m.group(idx++));
@@ -2656,7 +2655,7 @@
                     f.add(Flags.UPPERCASE);
             }

-            conversion(m.group(idx++));
+            conversion(m.group(idx));

             if (dt)
                 checkDateTime();
@@ -2811,9 +2810,9 @@

         private void printString(Object arg, Locale l) throws IOException {
             if (arg instanceof Formattable) {
-                Formatter fmt = formatter;
-                if (formatter.locale() != l)
-                    fmt = new Formatter(formatter.out(), l);
+                Formatter fmt = Formatter.this;
+                if (fmt.locale() != l)
+                    fmt = new Formatter(fmt.out(), l);
                 ((Formattable)arg).formatTo(fmt, f.valueOf(), width,
precision);
             } else {
                 if (f.contains(Flags.ALTERNATE))

FormatConversion is an inner class, so already has
a pointer to its enclosing instance - no need to provide
an additional explicit one.

Martin

>
>
> The rest looks fine.
>
> sherman
>
>
> Martin Buchholz wrote:
>>
>> Hi Sherman,
>>
>> I'd like you to do a code review.
>>
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/Formatter.parse/
>> <http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk7/Formatter.parse/>
>>
>> Synopsis: Optimize Formatter.parse (including String.printf)
>> Description:
>> Formatter is not as efficient as it could be.
>> Here's a low-hanging fruit optimization
>> that creates fewer objects,
>> and uses the idiom
>> return al.toArray(new FormatString[al.size()]);
>>  I'm sure additional optimizations are possible.
>>
>> Results: about 10-20% faster on in-house microbenchmarks of
>> String.printf
>>
>> Martin
>
>