8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)
peter.levart at gmail.com
Wed Mar 27 08:01:31 UTC 2013
On 03/26/2013 10:37 PM, Mandy Chung wrote:
> Hi Peter,
> I'm glad that you observe similar performance improvement without the
> need of method handles. I reviewed this version and realize that the
> map from j.u.l.Level object to LevelEnum can be removed entirely.
> sun.util.logging.PlatformLogger is an internal API that should only be
> used by JDK. The static final int fields representing the level value
> can be changed to be static final LevelEnum type instead. I checked
> the JDK code that uses PlatformLogger and no code will be impacted by
> the change of the type of these static fields. So it removes the need
> to map from an integer value to LevelEnum. Mapping from a j.u.l.Level
> to LevelEnum is trivial - the name of the LevelEnum is the same as
> j.u.l.Level (e.g. LevelEnum.FINEST and Level.FINEST), you can call
> LoggingSupport.getLevelName(javaLevel) to find its name and
> LevelEnum.valueOf(levelName) returns the LevelEnum instance. However,
> this would require more changes - basically the methods taking "int
> level" as a parameter would be modified to take LevelEnum and
> getLevel() would return LevelEnum too. I think it's worth doing this
> cleanup to get rid of the unnecessary conversion from int -> enum ->
> j.u.l.Level and vice versa. I also recommend to rename LevelEnum to
> Level which is an inner class of PlatformLogger. What do you think of
> this alternative to get rid of the map?
This is exactly what I was thinking too, but didn't know who uses the
API... So if the whole JDK builds with API changes, we are fine? No
> Some other comments of your patch:
> - it renames the class JavaLogger to JavaLoggerProxy and the variable
> from logger to loggerProxy. I'm fine with that.
> - L162: JavaLoggerProxy.init() to force load of the class which leads
> to separating the initialization of LevelEnum.javaLevel in a new
> JavaLevel class. The JavaLevel utility methods are not needed if we
> change the static final fields to LevelEnum.
> Have you tried:
> false, PlatformLogger.getClassLoader());
> would this give you the same performance improvement? If so, you can
> keep the static initialization in the JavaLoggerProxy class.
Another way is: JavaLoggerProxy.class.getName(); I think it should. If
JIT only depends on loaded classes (and not whether they have already
been initialized or not). Will try...
> Thanks for expanding the PlatformLoggerTest to cover additional test
> cases. It's good that you compare the value of the PlatformLogger
> static final fields with j.u.l.Level.intValue(). You now can use the
> API to compare the LevelEnum with Level rather than reflection.
> Perhaps you can add the getName() and intValue() methods in LevelEnum
> class (just a thought).
Many more simplifications are possible if we change the API. Stay tuned.
> On 3/25/13 9:31 AM, Peter Levart wrote:
>> Well, Laurent, Mandy,
>> It turns out that the dispatch speed-up (or lack of slow-down to be
>> precise) is possible without MethodHandles too. Maybe some VM guru
>> could shed some light on this, but the following is currently the
>> fastest variant:
>> What I did is a simple "if" deciding between two call-sites, making
>> sure that each is only dispatching to single class. This only works,
>> if both classes (LoggerProxy and JavaLoggerProxy) are loaded in
>> advance, before 1st invocation on any of them is made (might be that
>> using MethodHandles forced initialization of both classes beforehand
>> and hence the speed-up). If I don't load JavaLoggerProxy before
>> warming-up with LoggerProxy, then when j.u.logging is enabled, speed
>> drops for a factor of almost 4 and never catches up even after very
>> long time.
>> This pre-loading also helps for a normal single call site dispatch
>> that dispatches to two distinct classes, but the speed 1st drops when
>> the class changes, and only catches-up after several billions of
>> iterations (30s tight loop on i7).
>> Now, because JavaLoggerProxy is initialized early, I had to move the
>> initialization of j.u.logging.Level objects and mappings to a
>> separate class "JavaLevel".
>> Here are the benchmark results for this last iteration...
More information about the core-libs-dev