logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Fwd: logging-log4j2 git commit: refactored ParameterizedMessage::recursiveDeepToString into smaller methods to enable inlining: after running a test with -XX:+PrintCompilation -XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining I found "...ParameterizedMessage:
Date Sun, 16 Aug 2015 21:01:47 GMT
The kind of long if/else's below begs for some switch or map lookup,
maybe...

Gary

---------- Forwarded message ----------
From: <rpopma@apache.org>
Date: Sun, Aug 16, 2015 at 7:26 AM
Subject: logging-log4j2 git commit: refactored
ParameterizedMessage::recursiveDeepToString into smaller methods to enable
inlining: after running a test with -XX:+PrintCompilation
-XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining I found
"...ParameterizedMessage:
To: commits@logging.apache.org


Repository: logging-log4j2
Updated Branches:
  refs/heads/master ceb0a6208 -> 011f2c787


refactored ParameterizedMessage::recursiveDeepToString into smaller
methods to enable inlining:
after running a test with -XX:+PrintCompilation
-XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining
I found "...ParameterizedMessage::recursiveDeepToString (836 bytes) hot
method too big".

Array, Map or Collection parameter handling can be further optimized but
I am assuming these are rarely hot. The code handling other types of
parameters is inlined after ~400 invocations.

Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit:
http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/011f2c78
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/011f2c78
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/011f2c78

Branch: refs/heads/master
Commit: 011f2c787e62d522a6789ee35315f4d90629608e
Parents: ceb0a62
Author: rpopma <rpopma@apache.org>
Authored: Sun Aug 16 23:26:25 2015 +0900
Committer: rpopma <rpopma@apache.org>
Committed: Sun Aug 16 23:26:25 2015 +0900

----------------------------------------------------------------------
 .../log4j/message/ParameterizedMessage.java     | 233 +++++++++++--------
 1 file changed, 140 insertions(+), 93 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/011f2c78/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
----------------------------------------------------------------------
diff --git
a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
index 2adf3b7..6b71eff 100644
---
a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
+++
b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
@@ -394,123 +394,170 @@ public class ParameterizedMessage implements
Message {
      * @param dejaVu a list of container identities that were already used.
      */
     private static void recursiveDeepToString(final Object o, final
StringBuilder str, final Set<String> dejaVu) {
-        if (o == null) {
-            str.append("null");
+        if (appendStringDateOrNull(o, str)) {
             return;
         }
-        if (o instanceof String) {
-            str.append(o);
-            return;
+        if (isMaybeRecursive(o)) {
+            appendPotentiallyRecursiveValue(o, str, dejaVu);
+        } else {
+            tryObjectToString(o, str);
         }
+    }

+    private static boolean appendStringDateOrNull(final Object o, final
StringBuilder str) {
+        if (o == null || o instanceof String) {
+            str.append(String.valueOf(o));
+            return true;
+        }
+        return appendDate(o, str);
+    }
+
+    private static boolean appendDate(final Object o, final StringBuilder
str) {
+        if (!(o instanceof Date)) {
+            return false;
+        }
+        final Date date = (Date) o;
+        final SimpleDateFormat format = getSimpleDateFormat();
+        str.append(format.format(date));
+        return true;
+    }
+
+    private static SimpleDateFormat getSimpleDateFormat() {
+        // I'll leave it like this for the moment... this could probably
be optimized using ThreadLocal...
+        return new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ");
+    }
+
+    /**
+     * Returns {@code true} if the specified object is an array, a Map or
a Collection.
+     */
+    private static boolean isMaybeRecursive(final Object o) {
+        return o.getClass().isArray() || o instanceof Map || o instanceof
Collection;
+    }
+
+    private static void appendPotentiallyRecursiveValue(final Object o,
final StringBuilder str,
+            final Set<String> dejaVu) {
         final Class<?> oClass = o.getClass();
         if (oClass.isArray()) {
-            if (oClass == byte[].class) {
-                str.append(Arrays.toString((byte[]) o));
-            } else if (oClass == short[].class) {
-                str.append(Arrays.toString((short[]) o));
-            } else if (oClass == int[].class) {
-                str.append(Arrays.toString((int[]) o));
-            } else if (oClass == long[].class) {
-                str.append(Arrays.toString((long[]) o));
-            } else if (oClass == float[].class) {
-                str.append(Arrays.toString((float[]) o));
-            } else if (oClass == double[].class) {
-                str.append(Arrays.toString((double[]) o));
-            } else if (oClass == boolean[].class) {
-                str.append(Arrays.toString((boolean[]) o));
-            } else if (oClass == char[].class) {
-                str.append(Arrays.toString((char[]) o));
-            } else {
-                // special handling of container Object[]
-                final String id = identityToString(o);
-                if (dejaVu.contains(id)) {
-
str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX);
-                } else {
-                    dejaVu.add(id);
-                    final Object[] oArray = (Object[]) o;
-                    str.append('[');
-                    boolean first = true;
-                    for (final Object current : oArray) {
-                        if (first) {
-                            first = false;
-                        } else {
-                            str.append(", ");
-                        }
-                        recursiveDeepToString(current, str, new
HashSet<>(dejaVu));
-                    }
-                    str.append(']');
-                }
-                //str.append(Arrays.deepToString((Object[]) o));
-            }
+            appendArray(o, str, dejaVu, oClass);
         } else if (o instanceof Map) {
-            // special handling of container Map
-            final String id = identityToString(o);
-            if (dejaVu.contains(id)) {
-
str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX);
-            } else {
-                dejaVu.add(id);
-                final Map<?, ?> oMap = (Map<?, ?>) o;
-                str.append('{');
-                boolean isFirst = true;
-                for (final Object o1 : oMap.entrySet()) {
-                    final Map.Entry<?, ?> current = (Map.Entry<?, ?>) o1;
-                    if (isFirst) {
-                        isFirst = false;
-                    } else {
-                        str.append(", ");
-                    }
-                    final Object key = current.getKey();
-                    final Object value = current.getValue();
-                    recursiveDeepToString(key, str, new HashSet<>(dejaVu));
-                    str.append('=');
-                    recursiveDeepToString(value, str, new
HashSet<>(dejaVu));
-                }
-                str.append('}');
-            }
+            appendMap(o, str, dejaVu);
         } else if (o instanceof Collection) {
-            // special handling of container Collection
+            appendCollection(o, str, dejaVu);
+        }
+    }
+
+    private static void appendArray(final Object o, final StringBuilder
str, final Set<String> dejaVu,
+            final Class<?> oClass) {
+        if (oClass == byte[].class) {
+            str.append(Arrays.toString((byte[]) o));
+        } else if (oClass == short[].class) {
+            str.append(Arrays.toString((short[]) o));
+        } else if (oClass == int[].class) {
+            str.append(Arrays.toString((int[]) o));
+        } else if (oClass == long[].class) {
+            str.append(Arrays.toString((long[]) o));
+        } else if (oClass == float[].class) {
+            str.append(Arrays.toString((float[]) o));
+        } else if (oClass == double[].class) {
+            str.append(Arrays.toString((double[]) o));
+        } else if (oClass == boolean[].class) {
+            str.append(Arrays.toString((boolean[]) o));
+        } else if (oClass == char[].class) {
+            str.append(Arrays.toString((char[]) o));
+        } else {
+            // special handling of container Object[]
             final String id = identityToString(o);
             if (dejaVu.contains(id)) {

 str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX);
             } else {
                 dejaVu.add(id);
-                final Collection<?> oCol = (Collection<?>) o;
+                final Object[] oArray = (Object[]) o;
                 str.append('[');
-                boolean isFirst = true;
-                for (final Object anOCol : oCol) {
-                    if (isFirst) {
-                        isFirst = false;
+                boolean first = true;
+                for (final Object current : oArray) {
+                    if (first) {
+                        first = false;
                     } else {
                         str.append(", ");
                     }
-                    recursiveDeepToString(anOCol, str, new
HashSet<>(dejaVu));
+                    recursiveDeepToString(current, str, new
HashSet<>(dejaVu));
                 }
                 str.append(']');
             }
-        } else if (o instanceof Date) {
-            final Date date = (Date) o;
-            final SimpleDateFormat format = new
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ");
-            // I'll leave it like this for the moment... this could
probably be optimized using ThreadLocal...
-            str.append(format.format(date));
+            //str.append(Arrays.deepToString((Object[]) o));
+        }
+    }
+
+    private static void appendMap(final Object o, final StringBuilder str,
final Set<String> dejaVu) {
+        // special handling of container Map
+        final String id = identityToString(o);
+        if (dejaVu.contains(id)) {
+
str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX);
         } else {
-            // it's just some other Object, we can only use toString().
-            try {
-                str.append(o.toString());
-            } catch (final Throwable t) {
-                str.append(ERROR_PREFIX);
-                str.append(identityToString(o));
-                str.append(ERROR_SEPARATOR);
-                final String msg = t.getMessage();
-                final String className = t.getClass().getName();
-                str.append(className);
-                if (!className.equals(msg)) {
-                    str.append(ERROR_MSG_SEPARATOR);
-                    str.append(msg);
+            dejaVu.add(id);
+            final Map<?, ?> oMap = (Map<?, ?>) o;
+            str.append('{');
+            boolean isFirst = true;
+            for (final Object o1 : oMap.entrySet()) {
+                final Map.Entry<?, ?> current = (Map.Entry<?, ?>) o1;
+                if (isFirst) {
+                    isFirst = false;
+                } else {
+                    str.append(", ");
+                }
+                final Object key = current.getKey();
+                final Object value = current.getValue();
+                recursiveDeepToString(key, str, new HashSet<>(dejaVu));
+                str.append('=');
+                recursiveDeepToString(value, str, new HashSet<>(dejaVu));
+            }
+            str.append('}');
+        }
+    }
+
+    private static void appendCollection(final Object o, final
StringBuilder str, final Set<String> dejaVu) {
+        // special handling of container Collection
+        final String id = identityToString(o);
+        if (dejaVu.contains(id)) {
+
str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX);
+        } else {
+            dejaVu.add(id);
+            final Collection<?> oCol = (Collection<?>) o;
+            str.append('[');
+            boolean isFirst = true;
+            for (final Object anOCol : oCol) {
+                if (isFirst) {
+                    isFirst = false;
+                } else {
+                    str.append(", ");
                 }
-                str.append(ERROR_SUFFIX);
+                recursiveDeepToString(anOCol, str, new HashSet<>(dejaVu));
             }
+            str.append(']');
+        }
+    }
+
+    private static void tryObjectToString(final Object o, final
StringBuilder str) {
+        // it's just some other Object, we can only use toString().
+        try {
+            str.append(o.toString());
+        } catch (final Throwable t) {
+            handleErrorInObjectToString(o, str, t);
+        }
+    }
+
+    private static void handleErrorInObjectToString(final Object o, final
StringBuilder str, final Throwable t) {
+        str.append(ERROR_PREFIX);
+        str.append(identityToString(o));
+        str.append(ERROR_SEPARATOR);
+        final String msg = t.getMessage();
+        final String className = t.getClass().getName();
+        str.append(className);
+        if (!className.equals(msg)) {
+            str.append(ERROR_MSG_SEPARATOR);
+            str.append(msg);
         }
+        str.append(ERROR_SUFFIX);
     }

     /**




-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Mime
View raw message