You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2022/03/14 21:04:48 UTC

[logging-log4j2] 08/17: LOG4J2-3393 Avoid StringBuilderEncoder thread-local allocation in JsonTemplateLayout.

This is an automated email from the ASF dual-hosted git repository.

vy pushed a commit to branch LOG4J2-3393
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 8b12d69e2afb393a7360dbedc70dc9238fabc021
Author: Volkan Yazici <vo...@yazi.ci>
AuthorDate: Sun Feb 20 21:34:34 2022 +0100

    LOG4J2-3393 Avoid StringBuilderEncoder thread-local allocation in JsonTemplateLayout.
---
 .../log4j/core/layout/StringBuilderEncoder.java    |  9 ++--
 .../log4j/core/layout/TextEncoderHelper.java       |  6 +--
 .../layout/template/json/JsonTemplateLayout.java   | 56 +++++++++++++++++-----
 3 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/StringBuilderEncoder.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/StringBuilderEncoder.java
index fb02393..d300a74 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/StringBuilderEncoder.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/StringBuilderEncoder.java
@@ -31,7 +31,6 @@ import org.apache.logging.log4j.status.StatusLogger;
  */
 public class StringBuilderEncoder implements Encoder<StringBuilder> {
 
-    private static final int DEFAULT_BYTE_BUFFER_SIZE = 8 * 1024;
     /**
      * This ThreadLocal uses raw and inconvenient Object[] to store three heterogeneous objects (CharEncoder, CharBuffer
      * and ByteBuffer) instead of a custom class, because it needs to contain JDK classes, no custom (Log4j) classes.
@@ -49,7 +48,7 @@ public class StringBuilderEncoder implements Encoder<StringBuilder> {
     private final int byteBufferSize;
 
     public StringBuilderEncoder(final Charset charset) {
-        this(charset, Constants.ENCODER_CHAR_BUFFER_SIZE, DEFAULT_BYTE_BUFFER_SIZE);
+        this(charset, Constants.ENCODER_CHAR_BUFFER_SIZE, Constants.ENCODER_BYTE_BUFFER_SIZE);
     }
 
     public StringBuilderEncoder(final Charset charset, final int charBufferSize, final int byteBufferSize) {
@@ -67,7 +66,7 @@ public class StringBuilderEncoder implements Encoder<StringBuilder> {
             final ByteBuffer byteBuffer = (ByteBuffer) threadLocalState[2];
             TextEncoderHelper.encodeText(charsetEncoder, charBuffer, byteBuffer, source, destination);
         } catch (final Exception ex) {
-            logEncodeTextException(ex, source, destination);
+            logEncodeTextException(ex, source);
             TextEncoderHelper.encodeTextFallBack(charset, source, destination);
         }
     }
@@ -90,8 +89,8 @@ public class StringBuilderEncoder implements Encoder<StringBuilder> {
         return threadLocalState;
     }
 
-    private void logEncodeTextException(final Exception ex, final StringBuilder text,
-            final ByteBufferDestination destination) {
+    private void logEncodeTextException(final Exception ex, final StringBuilder text) {
         StatusLogger.getLogger().error("Recovering from StringBuilderEncoder.encode('{}') error: {}", text, ex, ex);
     }
+
 }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/TextEncoderHelper.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/TextEncoderHelper.java
index 327386f..4191f57 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/TextEncoderHelper.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/TextEncoderHelper.java
@@ -48,11 +48,9 @@ public class TextEncoderHelper {
      * @param byteBuf thread-local buffer to temporarily hold converted bytes before copying them to the destination
      * @param text the text to convert and write to the destination
      * @param destination the destination to write the bytes to
-     * @throws CharacterCodingException if conversion failed
      */
-    static void encodeText(final CharsetEncoder charsetEncoder, final CharBuffer charBuf, final ByteBuffer byteBuf,
-            final StringBuilder text, final ByteBufferDestination destination)
-            throws CharacterCodingException {
+    public static void encodeText(final CharsetEncoder charsetEncoder, final CharBuffer charBuf, final ByteBuffer byteBuf,
+            final StringBuilder text, final ByteBufferDestination destination) {
         charsetEncoder.reset();
         if (text.length() > charBuf.capacity()) {
             encodeChunkedText(charsetEncoder, charBuf, byteBuf, text, destination);
diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayout.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayout.java
index cf8658e..a585bc5 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayout.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayout.java
@@ -26,10 +26,7 @@ import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
 import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
 import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
 import org.apache.logging.log4j.core.config.plugins.PluginElement;
-import org.apache.logging.log4j.core.layout.ByteBufferDestination;
-import org.apache.logging.log4j.core.layout.Encoder;
-import org.apache.logging.log4j.core.layout.LockingStringBuilderEncoder;
-import org.apache.logging.log4j.core.layout.StringBuilderEncoder;
+import org.apache.logging.log4j.core.layout.*;
 import org.apache.logging.log4j.core.util.Constants;
 import org.apache.logging.log4j.core.util.StringEncoder;
 import org.apache.logging.log4j.layout.template.json.resolver.EventResolverContext;
@@ -44,9 +41,14 @@ import org.apache.logging.log4j.layout.template.json.util.JsonWriter;
 import org.apache.logging.log4j.layout.template.json.util.Recycler;
 import org.apache.logging.log4j.layout.template.json.util.RecyclerFactory;
 import org.apache.logging.log4j.layout.template.json.util.Uris;
+import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.Strings;
 
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
 import java.nio.charset.Charset;
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.CodingErrorAction;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -204,19 +206,49 @@ public class JsonTemplateLayout implements StringLayout {
             final JsonWriter jsonWriter) {
         return () -> {
             final JsonWriter clonedJsonWriter = jsonWriter.clone();
-            final Encoder<StringBuilder> encoder = createStringBuilderEncoder(charset);
+            final Encoder<StringBuilder> encoder = new StringBuilderEncoder(charset);
             return new Context(clonedJsonWriter, encoder);
         };
     }
 
-    private static Encoder<StringBuilder> createStringBuilderEncoder(
-            final Charset charset) {
-        if (Constants.ENABLE_DIRECT_ENCODERS) {
-            return Constants.ENABLE_THREADLOCALS
-                    ? new StringBuilderEncoder(charset)
-                    : new LockingStringBuilderEncoder(charset);
+    /**
+     * {@link org.apache.logging.log4j.core.layout.StringBuilderEncoder} clone replacing thread-local allocations with instance fields.
+     */
+    private static final class StringBuilderEncoder implements Encoder<StringBuilder> {
+
+        private final Charset charset;
+
+        private final CharsetEncoder charsetEncoder;
+
+        private final CharBuffer charBuffer;
+
+        private final ByteBuffer byteBuffer;
+
+        private StringBuilderEncoder(final Charset charset) {
+            this.charset = charset;
+            this.charsetEncoder = charset
+                    .newEncoder()
+                    .onMalformedInput(CodingErrorAction.REPLACE)
+                    .onUnmappableCharacter(CodingErrorAction.REPLACE);
+            this.charBuffer = CharBuffer.allocate(Constants.ENCODER_CHAR_BUFFER_SIZE);
+            this.byteBuffer = ByteBuffer.allocate(Constants.ENCODER_BYTE_BUFFER_SIZE);
         }
-        return null;
+
+        @Override
+        public void encode(
+                final StringBuilder source,
+                final ByteBufferDestination destination) {
+            try {
+                TextEncoderHelper.encodeText(charsetEncoder, charBuffer, byteBuffer, source, destination);
+            } catch (final Exception error) {
+                StatusLogger
+                        .getLogger()
+                        .error("TextEncoderHelper.encodeText() failure", error);
+                final byte[] bytes = source.toString().getBytes(charset);
+                destination.writeBytes(bytes, 0, bytes.length);
+            }
+        }
+
     }
 
     @Override