From c5e34ddcefbda5835fa023adf3740aaef1b78091 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 12 Jul 2024 20:09:20 +0200 Subject: [PATCH] GH-524: Performance (minor): optimize buffer operations For ByteArrayBuffer, optimize the getInt()/putInt() operations to work directly in the buffer without copying into/from a local work area. Simplify some of the operations; in particular, avoid extending bytes to long and then shifting. Bug: https://github.com/apache/mina-sshd/issues/524 --- .../sshd/common/util/buffer/BufferUtils.java | 29 +++++--- .../common/util/buffer/ByteArrayBuffer.java | 66 +++++++++++++++++-- 2 files changed, 82 insertions(+), 13 deletions(-) diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java index b1c68a0fe..f585bc665 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java @@ -391,6 +391,25 @@ public static long readUInt(InputStream input, byte[] buf, int offset, int len) } } + /** + * @param buf A buffer holding a 32-bit signed integer in big endian format. + * @param off The offset of the data in the buffer + * @param len The available data length. Note: if more than 4 bytes are available, then only the + * first 4 bytes in the buffer will be used (starting at the specified offset) + * @return The integer value read + */ + public static int getInt(byte[] buf, int off, int len) { + if (len < Integer.BYTES) { + throw new IllegalArgumentException("Not enough data for an INT: required=" + Integer.BYTES + ", available=" + len); + } + + int i = (buf[off++] & 0xFF) << 24; + i |= (buf[off++] & 0xFF) << 16; + i |= (buf[off++] & 0xFF) << 8; + i |= buf[off] & 0xFF; + return i; + } + /** * @param buf A buffer holding a 32-bit unsigned integer in big endian format. Note: if more than 4 * bytes are available, then only the first 4 bytes in the buffer will be used @@ -409,15 +428,7 @@ public static long getUInt(byte... buf) { * @return The result as a {@code long} whose 32 high-order bits are zero */ public static long getUInt(byte[] buf, int off, int len) { - if (len < Integer.BYTES) { - throw new IllegalArgumentException("Not enough data for a UINT: required=" + Integer.BYTES + ", available=" + len); - } - - long l = (buf[off] << 24) & 0xff000000L; - l |= (buf[off + 1] << 16) & 0x00ff0000L; - l |= (buf[off + 2] << 8) & 0x0000ff00L; - l |= (buf[off + 3]) & 0x000000ffL; - return l; + return getInt(buf, off, len) & 0xFFFF_FFFFL; } public static long getLong(byte[] buf, int off, int len) { diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java index 3efa761ed..4a026adba 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java @@ -197,16 +197,60 @@ public Buffer clear(boolean wipeData) { @Override public byte getByte() { - ensureAvailable(Byte.BYTES); + if (rpos >= wpos) { + throw new BufferException("Underflow: no available bytes in buffer"); + } return data[rpos++]; } @Override public void putByte(byte b) { - ensureCapacity(Byte.BYTES); + if (wpos >= data.length) { + ensureCapacity(Byte.BYTES); + } data[wpos++] = b; } + @Override + public int getInt() { + if (rpos + Integer.BYTES > wpos) { + throw new BufferException("Underflow: not enough bytes in buffer, need " + Integer.BYTES); + } + int i = BufferUtils.getInt(data, rpos, Integer.BYTES); + rpos += Integer.BYTES; + return i; + } + + @Override + public void putInt(long i) { + ValidateUtils.checkTrue(((int) i) == i, "Invalid INT32 value: %d", i); + if (wpos + Integer.BYTES > data.length) { + ensureCapacity(Integer.BYTES); + } + BufferUtils.putUInt(i, data, wpos, Integer.BYTES); + wpos += Integer.BYTES; + } + + @Override + public long getUInt() { + if (rpos + Integer.BYTES > wpos) { + throw new BufferException("Underflow: not enough bytes in buffer, need " + Integer.BYTES); + } + long l = BufferUtils.getUInt(data, rpos, Integer.BYTES); + rpos += Integer.BYTES; + return l; + } + + @Override + public void putUInt(long i) { + ValidateUtils.checkTrue((i & 0xFFFF_FFFFL) == i, "Invalid UINT32 value: %d", i); + if (wpos + Integer.BYTES > data.length) { + ensureCapacity(Integer.BYTES); + } + BufferUtils.putUInt(i, data, wpos, Integer.BYTES); + wpos += Integer.BYTES; + } + @Override public int putBuffer(Readable buffer, boolean expand) { int required = expand ? buffer.available() : Math.min(buffer.available(), capacity()); @@ -219,16 +263,30 @@ public int putBuffer(Readable buffer, boolean expand) { @Override public void putBuffer(ByteBuffer buffer) { int required = buffer.remaining(); - ensureCapacity(required + Integer.SIZE); + ensureCapacity(required + Integer.BYTES); putUInt(required); buffer.get(data, wpos, required); wpos += required; } + @Override + public void putBytes(byte[] b, int off, int len) { + ValidateUtils.checkTrue(len >= 0, "Negative raw bytes length: %d", len); + if (wpos + Integer.BYTES + len > data.length) { + ensureCapacity(Integer.BYTES + len); + } + BufferUtils.putUInt(len, data, wpos, Integer.BYTES); + wpos += Integer.BYTES; + System.arraycopy(b, off, data, wpos, len); + wpos += len; + } + @Override public void putRawBytes(byte[] d, int off, int len) { ValidateUtils.checkTrue(len >= 0, "Negative raw bytes length: %d", len); - ensureCapacity(len); + if (wpos + len > data.length) { + ensureCapacity(len); + } System.arraycopy(d, off, data, wpos, len); wpos += len; }