You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ta...@apache.org on 2022/09/13 19:10:27 UTC

[qpid-proton-j] branch main updated: PROTON-2609 Remove buffer slices and duplicates from the codec

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

tabish pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton-j.git


The following commit(s) were added to refs/heads/main by this push:
     new fa744f39 PROTON-2609 Remove buffer slices and duplicates from the codec
fa744f39 is described below

commit fa744f39843a26908cabf47dca5f8354bc57d6dc
Author: Timothy Bish <ta...@gmail.com>
AuthorDate: Tue Sep 13 14:48:06 2022 -0400

    PROTON-2609 Remove buffer slices and duplicates from the codec
    
    The decoder creates ByteBuffer slices and duplicates for operations
    that really on need simple state tracking to set limits for gets
    when copying incoming buffers to byte arrays for String or Symbol
    decoding. We can reduce GC pressure under load by just removing
    those in the codec without and impact on the API.
---
 .../java/org/apache/qpid/proton/amqp/Binary.java   | 28 +++++++++++++++++-----
 .../org/apache/qpid/proton/codec/DecoderImpl.java  | 11 +++++++--
 .../org/apache/qpid/proton/codec/SymbolType.java   |  9 ++++---
 .../qpid/proton/engine/impl/FrameWriter.java       |  8 +------
 .../qpid/proton/codec/ListTypeCodecTest.java       |  5 ++++
 .../qpid/proton/codec/PropertiesCodecTest.java     |  5 ++++
 6 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/proton-j/src/main/java/org/apache/qpid/proton/amqp/Binary.java b/proton-j/src/main/java/org/apache/qpid/proton/amqp/Binary.java
index f65a1176..49a385b2 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/amqp/Binary.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/amqp/Binary.java
@@ -166,6 +166,11 @@ public final class Binary
     }
 
     public static Binary create(ReadableBuffer buffer)
+    {
+        return create(buffer, buffer.remaining());
+    }
+
+    public static Binary create(ReadableBuffer buffer, int length)
     {
         if (buffer == null)
         {
@@ -173,9 +178,14 @@ public final class Binary
         }
         else if (!buffer.hasArray())
         {
-            byte data[] = new byte [buffer.remaining()];
-            ReadableBuffer dup = buffer.duplicate();
-            dup.get(data);
+            final byte data[] = new byte [length];
+            final int oldPos = buffer.position();
+            try {
+                buffer.get(data, 0, length);
+            } finally {
+                buffer.position(oldPos);
+            }
+
             return new Binary(data);
         }
         else
@@ -192,9 +202,15 @@ public final class Binary
         }
         if (buffer.isDirect() || buffer.isReadOnly())
         {
-            byte data[] = new byte [buffer.remaining()];
-            ByteBuffer dup = buffer.duplicate();
-            dup.get(data);
+            final int length = buffer.remaining();
+            final byte data[] = new byte [length];
+            final int oldPos = buffer.position();
+            try {
+                buffer.get(data, 0, length);
+            } finally {
+                buffer.position(oldPos);
+            }
+
             return new Binary(data);
         }
         else
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java
index 12be5261..cfce5893 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java
@@ -1075,8 +1075,15 @@ public class DecoderImpl implements ByteBufferDecoder
 
     <V> V readRaw(TypeDecoder<V> decoder, int size)
     {
-        V decode = decoder.decode(this, _buffer.slice().limit(size));
-        _buffer.position(_buffer.position()+size);
+        final int oldLimit = _buffer.limit();
+        final V decode;
+
+        try {
+            decode = decoder.decode(this, _buffer.limit(_buffer.position() + size));
+        } finally {
+            _buffer.limit(oldLimit);
+        }
+
         return decode;
     }
 
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/SymbolType.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/SymbolType.java
index 6c89cba6..faee7b0b 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/SymbolType.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/SymbolType.java
@@ -44,14 +44,17 @@ public class SymbolType extends AbstractPrimitiveType<Symbol>
                 Symbol symbol = _symbolCache.get(buffer);
                 if (symbol == null)
                 {
-                    byte[] bytes = new byte[buffer.limit()];
+                    final byte[] bytes = new byte[buffer.remaining()];
                     buffer.get(bytes);
 
-                    String str = new String(bytes, ASCII_CHARSET);
-                    symbol = Symbol.getSymbol(str);
+                    symbol = Symbol.getSymbol(new String(bytes, ASCII_CHARSET));
 
                     _symbolCache.put(ReadableBuffer.ByteBufferReader.wrap(bytes), symbol);
                 }
+                else
+                {
+                    buffer.position(buffer.limit());
+                }
                 return symbol;
             }
         };
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameWriter.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameWriter.java
index 2ec13c16..6b689f90 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameWriter.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameWriter.java
@@ -158,13 +158,7 @@ class FrameWriter {
     private void logFrame(int channel, Object frameBody, ReadableBuffer payload, int payloadSize) {
         ProtocolTracer tracer = transport.getProtocolTracer();
         if (frameType == AMQP_FRAME_TYPE) {
-            ReadableBuffer originalPayload = null;
-            if (payload != null) {
-                originalPayload = payload.slice();
-                originalPayload.limit(payloadSize);
-            }
-
-            Binary payloadBin = Binary.create(originalPayload);
+            final Binary payloadBin = Binary.create(payload, payloadSize);
             FrameBody body = null;
             if (frameBody == null) {
                 body = EmptyFrame.INSTANCE;
diff --git a/proton-j/src/test/java/org/apache/qpid/proton/codec/ListTypeCodecTest.java b/proton-j/src/test/java/org/apache/qpid/proton/codec/ListTypeCodecTest.java
index b486fd15..1ca069fa 100644
--- a/proton-j/src/test/java/org/apache/qpid/proton/codec/ListTypeCodecTest.java
+++ b/proton-j/src/test/java/org/apache/qpid/proton/codec/ListTypeCodecTest.java
@@ -43,6 +43,11 @@ public class ListTypeCodecTest extends CodecTestSupport {
     private final int LARGE_SIZE = 1024;
     private final int SMALL_SIZE = 32;
 
+    @Test
+    public void testDecodeList() throws IOException {
+        doTestDecodeListSeries(1);
+    }
+
     @Test
     public void testDecodeSmallSeriesOfLists() throws IOException {
         doTestDecodeListSeries(SMALL_SIZE);
diff --git a/proton-j/src/test/java/org/apache/qpid/proton/codec/PropertiesCodecTest.java b/proton-j/src/test/java/org/apache/qpid/proton/codec/PropertiesCodecTest.java
index e6ef140f..00d91a16 100644
--- a/proton-j/src/test/java/org/apache/qpid/proton/codec/PropertiesCodecTest.java
+++ b/proton-j/src/test/java/org/apache/qpid/proton/codec/PropertiesCodecTest.java
@@ -37,6 +37,11 @@ public class PropertiesCodecTest extends CodecTestSupport {
     private final int LARGE_SIZE = 1024;
     private final int SMALL_SIZE = 32;
 
+    @Test
+    public void testDecodePropertiess() throws IOException {
+        doTestDecodePropertiesSeries(1);
+    }
+
     @Test
     public void testDecodeSmallSeriesOfPropertiess() throws IOException {
         doTestDecodePropertiesSeries(SMALL_SIZE);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org