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-...@openjdk.java.net and security-...@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
