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

[Mandy Chung]

On 4/9/13 12:37 AM, Laurent Bourgès wrote:
> 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
>
This is the patch you refer to:
http://jmmc.fr/~bourgesl/share/webrev-8010297.3/ 
<http://jmmc.fr/%7Ebourgesl/share/webrev-8010297.3/>

I agree that we should separate the fix to reduce the memory usage from 
the fix to convert JUL to PlatformLogger.  I skimmed on your patch - 
awt/swing uses PlatformLogger and your change looks fine.  You have also 
got Anthony's approval.  Other component security, jmx, snmp, etc are 
using JUL.  I would suggest to fix the use of PlatformLogger in your 
first patch and AWT/Swing is the main area that 8010297 is concerned 
about so that you can resolve 8010297 soon.

If you want to move ahead to fix use of JUL, you can send your patch to 
serviceability-dev at openjdk.java.net and security-dev at openjdk.java.net 
for the other areas to review and sponsor.

Hope this helps.
Mandy

> 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20130409/9e23f35f/attachment.html