8153666: Optimize Formatter.formatMessage

This patch brings a small optimization that removes needless synchronization in Formatter.formatMessage. It also fixes the code that decides whether to call MessageFormat, and brings a small clarification to the API documentation on the conditions when that will happen.

Reviewed-by: rriggs, martin
This commit is contained in:
Daniel Fuchs 2016-06-13 08:20:34 +01:00
parent c5b3a7c036
commit 4c69f47805
3 changed files with 31 additions and 17 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -101,7 +101,8 @@ public abstract class Formatter {
* formatting. * formatting.
* <ul> * <ul>
* <li>If there are no parameters, no formatter is used. * <li>If there are no parameters, no formatter is used.
* <li>Otherwise, if the string contains "{0" then * <li>Otherwise, if the string contains "{{@literal<digit>}"
* where {@literal <digit>} is in [0-9],
* java.text.MessageFormat is used to format the string. * java.text.MessageFormat is used to format the string.
* <li>Otherwise no formatting is performed. * <li>Otherwise no formatting is performed.
* </ul> * </ul>
@ -109,15 +110,14 @@ public abstract class Formatter {
* @param record the log record containing the raw message * @param record the log record containing the raw message
* @return a localized and formatted message * @return a localized and formatted message
*/ */
public synchronized String formatMessage(LogRecord record) { public String formatMessage(LogRecord record) {
String format = record.getMessage(); String format = record.getMessage();
java.util.ResourceBundle catalog = record.getResourceBundle(); java.util.ResourceBundle catalog = record.getResourceBundle();
if (catalog != null) { if (catalog != null) {
try { try {
format = catalog.getString(record.getMessage()); format = catalog.getString(format);
} catch (java.util.MissingResourceException ex) { } catch (java.util.MissingResourceException ex) {
// Drop through. Use record message as format // Drop through. Use record message as format
format = record.getMessage();
} }
} }
// Do the formatting. // Do the formatting.
@ -130,12 +130,17 @@ public abstract class Formatter {
// Is it a java.text style format? // Is it a java.text style format?
// Ideally we could match with // Ideally we could match with
// Pattern.compile("\\{\\d").matcher(format).find()) // Pattern.compile("\\{\\d").matcher(format).find())
// However the cost is 14% higher, so we cheaply check for // However the cost is 14% higher, so we cheaply use indexOf
// 1 of the first 4 parameters // and charAt to look for that pattern.
if (format.indexOf("{0") >= 0 || format.indexOf("{1") >=0 || int index = -1;
format.indexOf("{2") >=0|| format.indexOf("{3") >=0) { int fence = format.length() - 1;
while ((index = format.indexOf('{', index+1)) > -1) {
if (index >= fence) break;
char digit = format.charAt(index+1);
if (digit >= '0' & digit <= '9') {
return java.text.MessageFormat.format(format, parameters); return java.text.MessageFormat.format(format, parameters);
} }
}
return format; return format;
} catch (Exception ex) { } catch (Exception ex) {

View File

@ -153,7 +153,7 @@ public class SimpleFormatter extends Formatter {
* @return a formatted log record * @return a formatted log record
*/ */
@Override @Override
public synchronized String format(LogRecord record) { public String format(LogRecord record) {
ZonedDateTime zdt = ZonedDateTime.ofInstant( ZonedDateTime zdt = ZonedDateTime.ofInstant(
record.getInstant(), ZoneId.systemDefault()); record.getInstant(), ZoneId.systemDefault());
String source; String source;

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -23,7 +23,7 @@
/* /*
* @test * @test
* @bug 6381464 * @bug 6381464 8153666
* @summary Test the custom simple formatter output * @summary Test the custom simple formatter output
* *
* @run main/othervm SimpleFormatterFormat * @run main/othervm SimpleFormatterFormat
@ -60,13 +60,19 @@ public class SimpleFormatterFormat {
"test.foo", "test.foo",
"test.foo", "test.foo",
"test.bar", "test.bar",
"test.bar",
"test.bar",
"test.bar" "test.bar"
}; };
private static String[] messages = new String[] { private static String[] messages = new String[] {
"severe hello world", "severe hello world",
"warning lost connection", "warning lost connection",
"info welcome", "info welcome",
"warning exception thrown", "warning beware of traps",
"warning { {ok7} }",
// keep exception logging as last test case to avoid having
// to skip the exception stack trace in the output
"warning exception thrown"
}; };
private static void writeLogRecords(PrintStream logps) throws Exception { private static void writeLogRecords(PrintStream logps) throws Exception {
try { try {
@ -79,8 +85,11 @@ public class SimpleFormatterFormat {
Logger bar = Logger.getLogger("test.bar"); Logger bar = Logger.getLogger("test.bar");
bar.finest("Dummy message"); bar.finest("Dummy message");
bar.info(messages[2]); bar.info(messages[2]);
bar.log(Level.WARNING, messages[3], new IllegalArgumentException()); bar.log(Level.WARNING, "{0}", new Object[] { messages[3] });
bar.log(Level.WARNING, "warning '{' '{'{7}} }", new Object[] {"ok", "ok1", "ok2", "ok3", "ok4", "ok5", "ok6", "ok7", "ok8", "ok9", "ok10"});
// Keep this one last - as it also prints the exception stack trace...
bar.log(Level.WARNING, messages[messages.length-1], new IllegalArgumentException());
} finally { } finally {
logps.flush(); logps.flush();
logps.close(); logps.close();
@ -108,7 +117,7 @@ public class SimpleFormatterFormat {
Matcher m = p.matcher(line); Matcher m = p.matcher(line);
if (!m.matches()) { if (!m.matches()) {
throw new RuntimeException("Unexpected output format"); throw new RuntimeException("Unexpected output format: " + line);
} }
if (m.groupCount() != 3) { if (m.groupCount() != 3) {
throw new RuntimeException("Unexpected group count = " + throw new RuntimeException("Unexpected group count = " +