RFR: 8061472 String.format in DeferredAttr.DeferredTypeMap constructor leads to excessive object creation
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Thu Dec 18 22:08:53 UTC 2014
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);
>> }
>
More information about the compiler-dev
mailing list