8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)

[Mandy Chung]

Hi Peter, Laurent,

Good work and I like the solution.

On 3/21/13 9:00 AM, Peter Levart wrote:
>
> Hi Mandy,
>
> JavaLogger and LoggerProxy are only used from within PlatformLogger. 
> Wouldn't it be better to declare them private? Their usage (one or the 
> other) depends on the 'loggingSupport' flag that is initialized with 
> PlatformLogger class. Usage of JavaLogger or LoggerProxy outside 
> PlatformLogger would be wrong, I think.

Yes JavaLogger and LoggerProxy classes can be made private.

> Here's the new rev:
>
> http://dl.dropbox.com/u/101777488/jdk8-tl/PlatformLogger/webrev.05/index.html
>
> I removed the object() and forObject() methods and access the field 
> directly. I also renamed it to something more significant. forObject() 
> is not necessary, since it only had one call site and I just inlined 
> it. I also "took the opportunity". Now we're back to original + 9 LOC ...

I looked at it but not in great detail.  In general it's very good clean 
up.   The comment in line 124-132 is good information and since the code 
is evolving, I would suggest to take those count out.

valueOf is one common method name to find an instance of a given value.  
Perhaps LevelEnum.forValue can be renamed to LevelEnum.valueOf(int)?  It 
seems to be useful to add a static method LevelEnum.getLevel(int) to 
replace the calls to "LevelEnum.forValue(newLevel).julLevel".

The tests are in jdk/test/java/util/logging and 
jdk/test/sun/util/logging.  It'd be good to check if you want to extend 
jdk/test/sun/util/logging/PlatformLoggerTest.java to cover all levels 
(it's currently already checking several).

Thanks
Mandy