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

[Peter Levart]

On 03/22/2013 11:34 AM, Laurent Bourgès wrote:
> Peter,
>
>     You've beaten me! I have been preparing them too ;-) ...
>
>
> Ok I definitely stop working on the code and let you do it.

Ok. I'll make the final touches, incorporating also comments and changes 
from your code...

>
>     I also did some some renames, that I think make the code more
>     consistent:
>     - LevelEnum -> Level (the code is not dependent on
>     java.util.logging.Level, so the name can be reused, its private
>     anyway)
>     - julLevel -> javaLevel (javaLevel / JavaLogger)
>     - LevelEnum.forValue -> Level.valueOf (Mandy)
>     - JavaLogger.julLevelToEnum -> JavaLogger.javaLevelToLevel
>
>
> For consistency and clarity, I would prefer having following conventions:
> - int levelValue (= PlatformLevel as int) and not int level (conflict 
> with Level enum ...)

I think that PlatformLogger public API should stay as is. Public 
method's parameter names are just called 'level' and values of public 
constants are expected to be passed to them. There are only two places 
where 'level' is the name of a local variable of type Level (and not 
int) and at both places there is not conflict, since there's no 'int 
level' variable in scope.

I renamed LevelEnum to Level because the following most frequently used 
pattern looks strange:

     LevelEnum.javaLevel(int)

Neither parameter nor the result has anything to do with LevelEnum directly.

But if we move the javaLevel(int) method out of the Level enum into the 
JavaLogger, then Level can be renamed back to LevelEnum (or anything else?).


> - julLevel / julLogger: more explicit than javaLevel / javaLogger 
> (java means everything  ... but jul means java.util.logging and 
> javaLogger is in conflict with JavaLogger class)


But javaLogger is not in conflict with javaLevel. I suggest renaming 
JavaLoger class to JavaLoggerProxy, so we would have:

Object javaLogger // holding java.util.logging.Logger instance
Object javaLevel    // holding java.util.logging.Level instance

class JavaLoggerProxy  // a specialization of LoggerProxy, delegating to 
javaLogger

If 'jul' is a prefered prefix to 'java', I suggest renaming all 3: 
julLogger, julLevel, JulLoggerProxy. I don't have a preference for 
either, so perhaps Mandy, Laurent or anybody else can express them...


Regards, Peter

>
>     Other changes (to webrev.05):
>     - removed the occurrence counts in switch comments (as per Mandy's
>     suggestion)
>     - made LoggerProxy and JavaLogger private
>     - fixed double-read of volatile LoggerProxy.levelValue in
>     LoggerProxy.isLoggable()
>     - added static Level.javaLevel(int value) shortcut (Mandy)
>
>     I also updated the test to exercise the correctness of mappings.
>
>
> Well done.
>
> cheers,
> Laurent
>