RFR: 8061472 String.format in DeferredAttr.DeferredTypeMap constructor leads to excessive object creation

[Maurizio Cimadamore]

The fix looks ok - but it feels a bit odd for the AttrMode (which is a 
general enum) to be returning the name including the 'deferredTypeMap' 
string - as that's another class. Maybe a static 'EnumMap<AttrMode, 
String>' inside the DeferredAttrMap class would more encapsulated?

Maurizio

On 18/12/14 19:03, Vicente-Arturo Romero-Zaldivar wrote:
> Hi Joel,
>
> Thanks for fixing this issue. I wrote a patch, actually a dirty hack 
> :), like a year back to fix the same issue, while we were working on 
> the performance rush. For several reasons that patch was never applied 
> and was forgotten. So thank you very much for proposing a solution to 
> the problem.
>
> Approved with pleasure!
> Vicente
>
> On 12/18/2014 05:41 AM, Joel Borggrén-Franck wrote:
>> Hi,
>>
>> Jan suggested supplying the descriptive name with the enum 
>> constructor, which I agree reads better. New patch inline.
>>
>> cheers
>> /Joel
>>
>> diff -r 47926c290355 
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
>> --- 
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java 
>> Wed Dec 17 12:48:04 2014 -0800
>> +++ 
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java 
>> Thu Dec 18 14:28:34 2014 +0100
>> @@ -352,11 +352,24 @@
>>            * type are reversed after the speculative round finishes. 
>> This means the
>>            * expression tree will be left in a blank state.
>>            */
>> -        SPECULATIVE,
>> +        SPECULATIVE("deferredTypeMap[SPECULATIVE]"),
>>           /**
>>            * This is the plain type-checking mode. Produces 
>> side-effects on the underlying AST node
>>            */
>> -        CHECK
>> +        CHECK("deferredTypeMap[CHECK]");
>> +
>> +        /**
>> +         * Returns a descriptive name of this mode.
>> +         */
>> +        public String getDescriptiveName() {
>> +            return descriptiveName;
>> +        }
>> +
>> +        private final String descriptiveName;
>> +
>> +        private AttrMode(String descriptiveName) {
>> +            this.descriptiveName = descriptiveName;
>> +        }
>>       }
>>
>>       /**
>> @@ -849,7 +862,7 @@
>>           DeferredAttrContext deferredAttrContext;
>>
>>           protected DeferredTypeMap(AttrMode mode, Symbol msym, 
>> MethodResolutionPhase phase) {
>> -            super(String.format("deferredTypeMap[%s]", mode));
>> +            super(mode.getDescriptiveName());
>>               this.deferredAttrContext = new 
>> DeferredAttrContext(mode, msym, phase,
>>                       infer.emptyContext, emptyDeferredAttrContext, 
>> types.noWarnings);
>>           }
>