8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level (original) (raw)
Laurent Bourgès bourges.laurent at gmail.com
Fri Mar 22 09:05:51 UTC 2013
- Previous message: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)
- Next message: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Mandy & Peter,
Here is an update patch applying mandy's suggestions: http://jmmc.fr/~bourgesl/share/webrev-2-8010309/
Good work and I like the solution.
Thanks.
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.
Done.
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.
Fixed.
valueOf is one common method name to find an instance of a given value. Perhaps LevelEnum.forValue can be renamed to LevelEnum.valueOf(int)?
Done.
It seems to be useful to add a static method LevelEnum.getLevel(int) to replace the calls to "LevelEnum.forValue(newLevel).julLevel".
I agree I prefer using methods (field encapsulation) so I added:
/**
* java.util.logging.Level optionally initialized in
JavaLogger's static initializer
* and USED ONLY IN JavaLogger (only once java.util.logging
is available and enabled)
*/
private Object julLevel;
void setJulLevel(Object julLevel) {
this.julLevel = julLevel;
}
Object getJulLevel() {
return this.julLevel;
}
/**
* Return the java.util.logging.Level used only in JavaLogger
* @param levelValue PlatformLogger level as int
* @return java.util.logging.Level or null if
java.util.logging is not available and disabled
*/
static Object getJulLevel(int levelValue) {
return valueOf(levelValue).getJulLevel();
}
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).
I will now have a look ...
Regards, Laurent
- Previous message: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)
- Next message: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]