sun.awt.X11 logs still using String + (waste) (original) (raw)

Laurent Bourgès bourges.laurent at gmail.com
Tue Apr 9 07:37:48 UTC 2013


Mandy,

first I would like to have the given patch applied to OpenJDK 8 (+ JDK7u) as it fixes many problems:

In a second step, I can help somebody migrating JUL usages to PlatformLogger but it requires PlatformLogger API changes (logp to give class and method names)...

Other comments below:

Peter's idea is a good one to add a couple of convenient methods to take Object parameters that will avoid the creation of array instances. I'd also like to know what variants being used in the jdk to determine the pros and cons of the alternatives (this issue also applies to java.util.logging).

50% of given object parameters are created via method calls so it will not be enough.

Moreover, I prefer having always isLoggable() calls to avoid me looking and understanding special cases (more than 2 args or no string concat ...); besides, it would be easier to implement code checks testing missing isLoggable() calls vs conditional and specific checks (string concat, more then 2 args, method calls ...)

Finally, I prefer having shorter and clearer method names like isFine(), isFinest() instead of isLoggable(PlatformLogger.FINE) that is quite "ugly" ...

It was intentional to have PlatformLogger define only the useful methods necessary for jdk use. The goal of PlatformLogger was to provide a light-weight utility for jdk to log debugging messages and eliminate the dependency to java.util.logging and avoid the unnecessary j.u.logging initialization (as disabled by default). It was not a goal for PlatformLogger to be an exact mirror as java.util.logging.Logger. My advice is to tune PlatformLogger to provide API that helps the platform implementation while avoid bloating the API.

Agreed.

Maybe JUL Logger.logp(classname, methodname) usages should be rewritten to use PlatformLogger.getCallerInfo() instead: // Returns the caller's class and method's name; best effort // if cannot infer, return the logger's name. private String getCallerInfo() { String sourceClassName = null; String sourceMethodName = null;

frame. if (cname.equals(logClassName)) { lookingForLogger = false; } } else { if (!cname.equals(logClassName)) { // We've found the relevant frame. sourceClassName = cname; sourceMethodName = frame.getMethodName(); break; } } }

        if (sourceClassName != null) {
            return sourceClassName + " " + sourceMethodName;
        } else {
            return name;
        }
    }
}

Since you are touching some jdk files that use java.util.logging, would you like to contribute to convert them to use PlatformLogger: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7054233 It's perfectly fine to convert only some of them if not all.

I want to help but as you said it should be done in collaboration because it requires API changes (JMX, RMI ...) but I prefer after this patch is reviewed and submitted and in coming weeks.

Jim Gish is the owner of logging and will check with him if he has cycles to work with you on this.

Great.

Cheers, Laurent



More information about the core-libs-dev mailing list