I created LOG4J2-935 for this. 

Best regards,
Remko

Sent from my iPhone

> On 2015/01/12, at 23:44, Gary Gregory <[email protected]> wrote:
> 
>> On Mon, Jan 12, 2015 at 8:28 AM, Remko Popma <[email protected]> wrote:
>> 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. 
> 
> Good, then we'll be comparing apples to apples.
> 
> Gary
>  
>> 
>> 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 <[email protected]> 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: <[email protected]>
>>> Date: Mon, Jan 12, 2015 at 3:53 AM
>>> Subject: [03/18] logging-log4j2 git commit: added helper method #getBytes
>>> To: [email protected]
>>> 
>>> 
>>> 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 <[email protected]>
>>> Authored: Mon Jan 12 17:25:37 2015 +0900
>>> Committer: rpopma <[email protected]>
>>> 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() {
>>>      }
>>> 
>>> 
>>> 
>>> 
>>> 
>>> -- 
>>> E-Mail: [email protected] | [email protected] 
>>> Java Persistence with Hibernate, Second Edition
>>> JUnit in Action, Second Edition
>>> Spring Batch in Action
>>> Blog: http://garygregory.wordpress.com 
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
> 
> 
> 
> -- 
> E-Mail: [email protected] | [email protected] 
> Java Persistence with Hibernate, Second Edition
> JUnit in Action, Second Edition
> Spring Batch in Action
> Blog: http://garygregory.wordpress.com 
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

Reply via email to