Sure. I already did some explaining in commit
df69b3fb81b0c22d4700b0fbcc91dbcca039d07d

(LOG4J2-935: document reason for this logic in comments)

Will add more to class javadoc.


On Monday, 5 October 2015, Gary Gregory <[email protected]> wrote:

> Can you please add comments here, otherwise, it just looks like really
> weird code. Maybe in the class Javadoc explain the Charset vs. charset name
> difference.
>
> Gary
>
> On Sun, Oct 4, 2015 at 4:44 PM, <[email protected]
> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote:
>
>> Repository: logging-log4j2
>> Updated Branches:
>>   refs/heads/master 9144b404b -> 3f9740b33
>>
>>
>> LOG4J2-935 Performance improvement converting Strings to byte[] arrays.
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit:
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3f9740b3
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3f9740b3
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3f9740b3
>>
>> Branch: refs/heads/master
>> Commit: 3f9740b33f713bbbb351f2e8d4c667b1b88ed52b
>> Parents: 9144b40
>> Author: rpopma <[email protected]
>> <javascript:_e(%7B%7D,'cvml','[email protected]');>>
>> Authored: Mon Oct 5 01:44:05 2015 +0200
>> Committer: rpopma <[email protected]
>> <javascript:_e(%7B%7D,'cvml','[email protected]');>>
>> Committed: Mon Oct 5 01:44:05 2015 +0200
>>
>> ----------------------------------------------------------------------
>>  .../log4j/core/layout/AbstractStringLayout.java   | 18 +++++++++++++++---
>>  src/changes/changes.xml                           |  3 +++
>>  2 files changed, 18 insertions(+), 3 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f9740b3/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
>> index cd10881..3a5d39b 100644
>> ---
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
>> +++
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
>> @@ -16,6 +16,7 @@
>>   */
>>  package org.apache.logging.log4j.core.layout;
>>
>> +import java.io.UnsupportedEncodingException;
>>  import java.nio.charset.Charset;
>>  import java.nio.charset.StandardCharsets;
>>
>> @@ -40,6 +41,7 @@ public abstract class AbstractStringLayout extends
>> AbstractLayout<String> {
>>       */
>>      // TODO: Charset is not serializable. Implement read/writeObject() ?
>>      private final Charset charset;
>> +    private final String charsetName;
>>
>>      protected AbstractStringLayout(final Charset charset) {
>>          this(charset, null, null);
>> @@ -48,6 +50,7 @@ public abstract class AbstractStringLayout extends
>> AbstractLayout<String> {
>>      protected AbstractStringLayout(final Charset charset, final byte[]
>> header, final byte[] footer) {
>>          super(header, footer);
>>          this.charset = charset == null ? StandardCharsets.UTF_8 :
>> charset;
>> +        this.charsetName = this.charset.name();
>>      }
>>
>>      /**
>> @@ -59,7 +62,12 @@ public abstract class AbstractStringLayout extends
>> AbstractLayout<String> {
>>       */
>>      static byte[] toBytes(final String str, final Charset charset) {
>>          if (str != null) {
>> -            return str.getBytes(charset != null ? charset :
>> Charset.defaultCharset());
>> +            final Charset actual = charset != null ? charset :
>> Charset.defaultCharset();
>> +            try {
>> +                return str.getBytes(actual.name());
>> +            } catch (UnsupportedEncodingException e) {
>> +                return str.getBytes(actual);
>> +            }
>>          }
>>          return null;
>>      }
>> @@ -80,7 +88,11 @@ public abstract class AbstractStringLayout extends
>> AbstractLayout<String> {
>>      }
>>
>>      protected byte[] getBytes(final String s) {
>> -        return s.getBytes(charset);
>> +        try {
>> +            return s.getBytes(charsetName);
>> +        } catch (UnsupportedEncodingException e) {
>> +            return s.getBytes(charset);
>> +        }
>>      }
>>
>>      protected Charset getCharset() {
>> @@ -103,7 +115,7 @@ public abstract class AbstractStringLayout extends
>> AbstractLayout<String> {
>>       */
>>      @Override
>>      public byte[] toByteArray(final LogEvent event) {
>> -        return toSerializable(event).getBytes(charset);
>> +        return getBytes(toSerializable(event));
>>      }
>>
>>  }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f9740b3/src/changes/changes.xml
>> ----------------------------------------------------------------------
>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml
>> index 65f3a12..1ab8b9f 100644
>> --- a/src/changes/changes.xml
>> +++ b/src/changes/changes.xml
>> @@ -66,6 +66,9 @@
>>        <action issue="LOG4J2-1126" dev="ggregory" type="fix">
>>          Web site corrections and updates.
>>        </action>
>> +      <action issue="LOG4J2-935" dev="rpopma" type="update">
>> +        Performance improvement when converting Strings to byte[] arrays.
>> +      </action>
>>        <action issue="LOG4J2-1040" dev="ggregory" type="update">
>>          Update MongoDB driver from 2.13.3 to 3.0.4.
>>        </action>
>>
>>
>
>
> --
> E-Mail: [email protected]
> <javascript:_e(%7B%7D,'cvml','[email protected]');> | [email protected]
> <javascript:_e(%7B%7D,'cvml','[email protected]');>
> 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
>

Reply via email to