<AWT Dev> <Swing Dev> sun.awt.X11 logs still using String + (waste)

[Laurent Bourgès]

Mandy,

first I would like to have the given patch applied to OpenJDK 8 (+ JDK7u)
as it fixes many problems:
- string concatenations
- object creations (Object[] or other objects given as params)
- method calls in log statements

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;

*            JavaLangAccess access = SharedSecrets.getJavaLangAccess();
*            Throwable throwable = new Throwable();
            int depth = access.getStackTraceDepth(throwable);

            String logClassName = "sun.util.logging.PlatformLogger";
            boolean lookingForLogger = true;
            for (int ix = 0; ix < depth; ix++) {
                // Calling getStackTraceElement directly prevents the VM
                // from paying the cost of building the entire stack frame.
                StackTraceElement frame =
                    access.getStackTraceElement(throwable, ix);
                String cname = frame.getClassName();
                if (lookingForLogger) {
                    // Skip all frames until we have found the first logger
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/view_bug.do?bug_id=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