Martin,
Did you want to remove the commented out formatter?
//private Formatter formatter;
- Dave Schlosnagle
On Thu, Nov 5, 2009 at 6:14 PM, Martin Buchholz <marti...@google.com> wrote:
> On Wed, Nov 4, 2009 at 23:07, Xueming Shen <xueming.s...@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
>>
>>
>
