<AWT Dev> <Swing Dev> sun.awt.X11 logs still using String + (waste)
Anthony Petrov
anthony.petrov at oracle.com
Wed Apr 10 11:19:00 UTC 2013
Laurent, I'm not subscribed to those mailing list, too. So you could
send/forward your review request to the lists yourself - no difference
here. Note that I tried sending your message to net-dev@ in the past,
and even contacted the maintainer of the mailing list via a private
email, but I never got any response and my messages never got accepted.
That's the reason I've CC'ed jdk8-dev@ yesterday...
--
best regards,
Anthony
On 4/10/2013 13:01, Laurent Bourgès wrote:
> Anthony or Mandy,
> Could you ask JMX / Security groups for me to review my patch ?
>
> I am currently not registered to these mailing lists.
>
> Do you ask me to split the patch in two part: PlatformLogger on one side
> and Logger on the other side ?
>
> Laurent
>
> 2013/4/9 Mandy Chung <mandy.chung at oracle.com
> <mailto:mandy.chung at oracle.com>>
>
> 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
> <mailto:serviceability-dev at openjdk.java.net> and
> security-dev at openjdk.java.net <mailto: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
>>
>
>
More information about the core-libs-dev
mailing list