Gary,

Thanks for looking at this. 
I do have a benchmark, I think it's called StringEncodingBenchmark in log4j-perf (from memory, away from PC now). Java version did make a difference: java 8 is much faster and the difference between getBytes(Charset) and getBytes(String) was smaller. 

The benchmark does not have a try/catch for the getBytes(String) version. I'll try that next. 

I'll create a separate Jira ticket for this and post my benchmark results there; I agree this may belong in master regardless of LOG4J2-930.

Again, thanks for the feedback!
Remko

Sent from my iPhone

On 2015/01/12, at 22:01, Gary Gregory <garydgregory@gmail.com> wrote:


+            // PERFORMANCE-SENSITIVE CODE (called for every log event by most layouts).
+            // String.getBytes(Charset) creates a new StringEncoder instance
+            // every call and is slightly slower than String.getBytes(String)

Playing a bit of devil's advocate here:
This needs better docs and is not convincing IMO. Does the faster one use more memory? Do you have a benchmark? Does the Java version matter? Is the cost of setting up the try-catch accounted for  in the benchmark?

If this is better, it belongs in master with all call sites updated. Or, should we revert to the String arg style with a // comment where Charset is used?

Gary


---------- Forwarded message ----------
From: <rpopma@apache.org>
Date: Mon, Jan 12, 2015 at 3:53 AM
Subject: [03/18] logging-log4j2 git commit: added helper method #getBytes
To: commits@logging.apache.org


added helper method #getBytes

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

Branch: refs/heads/LOG4J2-930
Commit: b66c93ab6d43ebd315b1655ccb319be2519edbbe
Parents: 89a03fc
Author: rpopma <rpopma@apache.org>
Authored: Mon Jan 12 17:25:37 2015 +0900
Committer: rpopma <rpopma@apache.org>
Committed: Mon Jan 12 17:25:37 2015 +0900

----------------------------------------------------------------------
 .../logging/log4j/core/util/Charsets.java       | 32 +++++++++++++++-----
 1 file changed, 24 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b66c93ab/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Charsets.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Charsets.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Charsets.java
index 2ad66b3..07f1087 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Charsets.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Charsets.java
@@ -16,13 +16,14 @@
  */
 package org.apache.logging.log4j.core.util;

+import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;

 import org.apache.logging.log4j.status.StatusLogger;

 /**
- * Charset utilities. Contains the standard character sets guaranteed to be available on all implementations of the
- * Java platform. Parts adapted from JDK 1.7 (in particular, the {@code java.nio.charset.StandardCharsets} class).
+ * Charset utilities. Contains the standard character sets guaranteed to be available on all implementations of the Java
+ * platform. Parts adapted from JDK 1.7 (in particular, the {@code java.nio.charset.StandardCharsets} class).
  *
  * @see java.nio.charset.Charset
  */
@@ -62,8 +63,7 @@ public final class Charsets {
      * Returns a Charset, if possible the Charset for the specified {@code charsetName}, otherwise (if the specified
      * {@code charsetName} is {@code null} or not supported) this method returns the platform default Charset.
      *
-     * @param charsetName
-     *            name of the preferred charset or {@code null}
+     * @param charsetName name of the preferred charset or {@code null}
      * @return a Charset, not null.
      */
     public static Charset getSupportedCharset(final String charsetName) {
@@ -74,10 +74,8 @@ public final class Charsets {
      * Returns a Charset, if possible the Charset for the specified {@code charsetName}, otherwise (if the specified
      * {@code charsetName} is {@code null} or not supported) this method returns the platform default Charset.
      *
-     * @param charsetName
-     *            name of the preferred charset or {@code null}
-     * @param defaultCharset
-     *            returned if {@code charsetName} is null or is not supported.
+     * @param charsetName name of the preferred charset or {@code null}
+     * @param defaultCharset returned if {@code charsetName} is null or is not supported.
      * @return a Charset, never null.
      */
     public static Charset getSupportedCharset(final String charsetName, final Charset defaultCharset) {
@@ -95,6 +93,24 @@ public final class Charsets {
         return charset;
     }

+    /**
+     * Returns a sequence of bytes that encodes the specified String, using the named Charset.
+     *
+     * @param txt the string to encode
+     * @param charset the {@code Charset} to use
+     * @return the encoded String
+     */
+    public static byte[] getBytes(final String txt, final Charset charset) {
+        try {
+            // PERFORMANCE-SENSITIVE CODE (called for every log event by most layouts).
+            // String.getBytes(Charset) creates a new StringEncoder instance
+            // every call and is slightly slower than String.getBytes(String)
+            return txt.getBytes(charset.name());
+        } catch (UnsupportedEncodingException e) {
+            return txt.getBytes(charset);
+        }
+    }
+
     private Charsets() {
     }





--