From 9e109b449fa15d77cf0606c17f90e9e850ec9844 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Sun, 30 Mar 2025 21:01:42 -0700 Subject: [PATCH 01/18] Optimize BSON decoding - Removed redundant String copying when possible, replacing with view-based access - Added faster null-termination lookups to improve decoding performance --- bson/src/main/org/bson/ByteBuf.java | 20 + bson/src/main/org/bson/ByteBufNIO.java | 10 + .../main/org/bson/io/ByteBufferBsonInput.java | 60 ++- .../internal/connection/CompositeByteBuf.java | 10 + .../internal/connection/ResponseBuffers.java | 3 +- .../connection/netty/NettyByteBuf.java | 10 + .../connection/ByteBufferBsonInputeTest.java | 475 ++++++++++++++++++ 7 files changed, 573 insertions(+), 15 deletions(-) create mode 100644 driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java diff --git a/bson/src/main/org/bson/ByteBuf.java b/bson/src/main/org/bson/ByteBuf.java index e44a97dfc67..d0708b61894 100644 --- a/bson/src/main/org/bson/ByteBuf.java +++ b/bson/src/main/org/bson/ByteBuf.java @@ -136,6 +136,26 @@ public interface ByteBuf { */ byte[] array(); + /** + *

States whether this buffer is backed by an accessible byte array.

+ * + *

If this method returns {@code true} then the {@link #array()} and {@link #arrayOffset()} methods may safely be invoked.

+ * + * @return {@code true} if, and only if, this buffer is backed by an array and is not read-only + * @since 5.5 + */ + boolean hasArray(); + + /** + * Returns the offset of the first byte within the backing byte array of + * this buffer. + * + * @throws java.nio.ReadOnlyBufferException If this buffer is backed by an array but is read-only + * @throws UnsupportedOperationException if this buffer is not backed by an accessible array + * @since 5.5 + */ + int arrayOffset(); + /** * Returns this buffer's limit. * diff --git a/bson/src/main/org/bson/ByteBufNIO.java b/bson/src/main/org/bson/ByteBufNIO.java index 83bfa7d893a..bb949ce0860 100644 --- a/bson/src/main/org/bson/ByteBufNIO.java +++ b/bson/src/main/org/bson/ByteBufNIO.java @@ -108,6 +108,16 @@ public byte[] array() { return buf.array(); } + @Override + public boolean hasArray() { + return buf.hasArray(); + } + + @Override + public int arrayOffset() { + return buf.arrayOffset(); + } + @Override public int limit() { return buf.limit(); diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index a5a0e7a5421..1f553d02f77 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -127,10 +127,7 @@ public String readString() { @Override public String readCString() { - int mark = buffer.position(); - skipCString(); - int size = buffer.position() - mark; - buffer.position(mark); + int size = computeCStringLength(buffer.position()); return readString(size); } @@ -146,26 +143,61 @@ private String readString(final int size) { } return ONE_BYTE_ASCII_STRINGS[asciiByte]; // this will throw if asciiByte is negative } else { - byte[] bytes = new byte[size - 1]; + if (buffer.hasArray()) { + int position = buffer.position(); + int arrayOffset = buffer.arrayOffset(); + buffer.position(position + size - 1); + byte nullByte = buffer.get(); + if (nullByte != 0) { + throw new BsonSerializationException("Found a BSON string that is not null-terminated"); + } + return new String(buffer.array(), arrayOffset + position, size - 1, StandardCharsets.UTF_8); + } + + byte[] bytes = new byte[size]; buffer.get(bytes); - byte nullByte = buffer.get(); - if (nullByte != 0) { - throw new BsonSerializationException("Found a BSON string that is not null-terminated"); + if (bytes[size - 1] != 0) { + throw new BsonSerializationException("BSON string not null-terminated"); } - return new String(bytes, StandardCharsets.UTF_8); + return new String(bytes, 0, size - 1, StandardCharsets.UTF_8); } } @Override public void skipCString() { ensureOpen(); - boolean checkNext = true; - while (checkNext) { - if (!buffer.hasRemaining()) { - throw new BsonSerializationException("Found a BSON string that is not null-terminated"); + int pos = buffer.position(); + int length = computeCStringLength(pos); + buffer.position(pos + length); + } + + public int computeCStringLength(int prevPos) { + ensureOpen(); + //TODO should we fall back to byte-by-byte search on JDK 8? ByteBuffer does not use Unsafe internally + int pos = prevPos; + int limit = buffer.limit(); + while (limit - pos >= Long.BYTES) { + long word = buffer.getLong(pos); + long mask = word - 0x0101010101010101L; + mask &= ~word; + mask &= 0x8080808080808080L; + if (mask != 0) { + // first null terminator found in the Little Endian long + int offset = Long.numberOfTrailingZeros(mask) >>> 3; + // Found the null at pos + offset; reset buffer's position. + return (pos - prevPos) + offset + 1; + } + pos += 8; + } + + // Process remaining bytes one-by-one. + while (pos < limit) { + if (buffer.get(pos++) == 0) { + return (pos - prevPos); } - checkNext = buffer.get() != 0; } + buffer.position(pos); + throw new BsonSerializationException("Found a BSON string that is not null-terminated"); } @Override diff --git a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java index fa8cde2e517..ca2ef40cc31 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java @@ -213,6 +213,16 @@ public byte[] array() { throw new UnsupportedOperationException("Not implemented yet!"); } + @Override + public boolean hasArray() { + return false; + } + + @Override + public int arrayOffset() { + throw new UnsupportedOperationException("Not implemented yet!"); + } + @Override public ByteBuf limit(final int newLimit) { if (newLimit < 0 || newLimit > capacity()) { diff --git a/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java b/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java index e984862fe0f..7dd897ab556 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java @@ -59,7 +59,8 @@ T getResponseDocument(final int messageId, final Decode * @return a read-only buffer containing the response body */ public ByteBuf getBodyByteBuffer() { - return bodyByteBuffer.asReadOnly(); + //TODO any side-effects to make it read-only? + return bodyByteBuffer; } public void reset() { diff --git a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java index 074e77de04f..d1e0f43462e 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java @@ -100,6 +100,16 @@ public byte[] array() { return proxied.array(); } + @Override + public boolean hasArray() { + return proxied.hasArray(); + } + + @Override + public int arrayOffset() { + return proxied.arrayOffset(); + } + @Override public int limit() { if (isWriting) { diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java new file mode 100644 index 00000000000..6226052e71e --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java @@ -0,0 +1,475 @@ +package com.mongodb.internal.connection; + +import com.google.common.primitives.Ints; +import com.mongodb.internal.connection.netty.NettyByteBuf; +import io.netty.buffer.PooledByteBufAllocator; +import org.bson.BsonSerializationException; +import org.bson.ByteBuf; +import org.bson.ByteBufNIO; +import org.bson.io.ByteBufferBsonInput; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +import static java.lang.Character.MAX_CODE_POINT; +import static java.lang.Character.MAX_LOW_SURROGATE; +import static java.lang.Character.MIN_HIGH_SURROGATE; +import static java.lang.Integer.reverseBytes; +import static java.lang.String.join; +import static java.util.Collections.nCopies; +import static java.util.stream.Collectors.toList; +import static java.util.stream.IntStream.range; +import static java.util.stream.IntStream.rangeClosed; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + + +class ByteBufferBsonInputTest { + + private static final List ALL_CODE_POINTS_EXCLUDING_SURROGATES = Stream.concat( + range(1, MIN_HIGH_SURROGATE).boxed(), + rangeClosed(MAX_LOW_SURROGATE + 1, MAX_CODE_POINT).boxed()) + .filter(i -> i < 128 || i % 10 == 0) // only subset of code points to speed up testing + .collect(toList()); + + static Stream bufferProviders() { + return Stream.of( + size -> new NettyByteBuf(PooledByteBufAllocator.DEFAULT.directBuffer(size)), + size -> new NettyByteBuf(PooledByteBufAllocator.DEFAULT.heapBuffer(size)), + new PowerOfTwoBufferPool(), + size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 5], 2, size).slice()), //different array offsets + size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 4], 3, size).slice()), //different array offsets + size -> new ByteBufNIO(ByteBuffer.allocate(size)) { + @Override + public boolean hasArray() { + return false; + } + + @Override + public byte[] array() { + return Assertions.fail("array() is called, when hasArray() returns false"); + } + + @Override + public int arrayOffset() { + return Assertions.fail("arrayOffset() is called, when hasArray() returns false"); + } + } + ); + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadEmptyString(final BufferProvider bufferProvider) { + // given + byte[] input = {1, 0, 0, 0, 0}; + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, input); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when + String result = bufferInput.readString(); + + // then + assertEquals("", result); + assertEquals(5, bufferInput.getPosition()); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadEmptyCString(final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{0}); + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when + String result = bufferInput.readCString(); + + // then + assertEquals("", result); + assertEquals(1, bufferInput.getPosition()); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadInvalidOneByteString(final BufferProvider bufferProvider) { + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{2, 0, 0, 0, (byte) 0xFF, 0}); + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readString(); + assertEquals("\uFFFD", result); + assertEquals(6, bufferInput.getPosition()); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadInvalidOneByteCString(final BufferProvider bufferProvider) { + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{-0x01, 0}); + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readCString(); + assertEquals("\uFFFD", result); + assertEquals(2, bufferInput.getPosition()); + } + } + + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadStringUptoBufferLimit(final BufferProvider bufferProvider) { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + byte[] expectedStringEncoding = getEncodedString(str); + + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readString(); + assertEquals(str, result); + assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + byte[] expectedStringEncoding = getEncodedString(str); + byte[] bufferBytes = mergeArrays( + expectedStringEncoding, + new byte[]{1, 2, 3} + ); + + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readString(); + assertEquals(str, result); + assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + + byte[] expectedStringEncoding = getEncodedString(str); + byte[] bufferBytes = mergeArrays( + new byte[]{1, 2, 3}, + expectedStringEncoding, + new byte[]{4, 5, 6} + ); + + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + buffer.position(3); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readString(); + assertEquals(str, result); + assertEquals(3 + expectedStringEncoding.length, bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadCStringUptoBufferLimit(final BufferProvider bufferProvider) { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + byte[] expectedStringEncoding = getEncodedCString(str); + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readCString(); + assertEquals(str, result); + assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + byte[] expectedStringEncoding = getEncodedCString(str); + byte[] bufferBytes = mergeArrays( + expectedStringEncoding, + new byte[]{1, 2, 3} + ); + + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + String result = bufferInput.readCString(); + assertEquals(str, result); + assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + //given + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + + byte[] expectedStringEncoding = getEncodedCString(str); + byte[] bufferBytes = mergeArrays( + new byte[]{1, 2, 3}, + expectedStringEncoding, + new byte[]{4, 5, 6} + ); + + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + buffer.position(3); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when / then + String result = bufferInput.readCString(); + assertEquals(str, result); + assertEquals(3 + expectedStringEncoding.length, bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldThrowIfCStringIsNotNullTerminatedSkip(final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{(byte) 0xe0, (byte) 0xa4, (byte) 0x80}); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + assertThrows(BsonSerializationException.class, stream::skipCString); + } + } + + + public static Stream nonNullTerminatedStringsWithBuffers() { + List arguments = new ArrayList<>(); + List collect = bufferProviders().collect(toList()); + for (BufferProvider bufferProvider : collect) { + arguments.add(Arguments.of(new byte[]{1, 0, 0, 0, 1}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{2, 0, 0, 0, 1, 3}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{3, 0, 0, 1, 2, 3}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{4, 0, 0, 0, 1, 2, 3, 4}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{8, 0, 0, 0, 2, 3, 4, 5, 6, 7, 8, 9}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{9, 0, 0, 0, 2, 3, 4, 5, 6, 7, 8, 9, 1}, bufferProvider)); + } + return arguments.stream(); + } + + @ParameterizedTest + @MethodSource("nonNullTerminatedStringsWithBuffers") + void shouldThrowIfStringIsNotNullTerminated(final byte[] nonNullTerminatedString, final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, nonNullTerminatedString); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + assertThrows(BsonSerializationException.class, stream::readString); + } + } + + public static Stream nonNullTerminatedCStringsWithBuffers() { + List arguments = new ArrayList<>(); + List collect = bufferProviders().collect(toList()); + for (BufferProvider bufferProvider : collect) { + arguments.add(Arguments.of(new byte[]{1}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{1, 2}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{1, 2, 3}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{1, 2, 3, 4}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{2, 3, 4, 5, 6, 7, 8, 9}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{2, 3, 4, 5, 6, 7, 8, 9, 1}, bufferProvider)); + } + return arguments.stream(); + } + + @ParameterizedTest + @MethodSource("nonNullTerminatedCStringsWithBuffers") + void shouldThrowIfCStringIsNotNullTerminated(final byte[] nonNullTerminatedCString, final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, nonNullTerminatedCString); + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + + // when / then + assertThrows(BsonSerializationException.class, bufferInput::readCString); + } + } + + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldThrowIfOneByteStringIsNotNullTerminated(final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{2, 0, 0, 0, 1}); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + assertThrows(BsonSerializationException.class, stream::readString); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldThrowIfOneByteCStringIsNotNullTerminated(final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{1}); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + assertThrows(BsonSerializationException.class, stream::readCString); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldThrowIfLengthOfBsonStringIsNotPositive(final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{-1, -1, -1, -1, 41, 42, 43, 0}); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + assertThrows(BsonSerializationException.class, stream::readString); + } + } + + public static Stream shouldSkipCStringWhenMultipleNullTerminationPresent() { + List arguments = new ArrayList<>(); + List collect = bufferProviders().collect(toList()); + for (BufferProvider bufferProvider : collect) { + arguments.add(Arguments.of(new byte[]{0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0x4b, 0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0x4b, 0x4c, 0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0x61, 0x76, 0x61, 0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0x61, 0x76, 0x61, 0x62, 0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0x61, 0x76, 0x61, 0x65, 0x62, 0x67, 0, 8, 0, 0, 0}, bufferProvider)); + arguments.add(Arguments.of(new byte[]{0x4a, 0, 8, 0, 0, 0}, bufferProvider)); + } + return arguments.stream(); + } + + @ParameterizedTest + @MethodSource() + void shouldSkipCStringWhenMultipleNullTerminationPresent(final byte[] cStringBytes, final BufferProvider bufferProvider) { + // given + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, cStringBytes); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + stream.skipCString(); + + assertEquals(cStringBytes.length - Integer.BYTES, stream.getPosition()); + assertEquals(8, stream.readInt32()); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadSkipCStringWhenMultipleNullTerminationPresentWithinBuffer(final BufferProvider bufferProvider) { + // given + byte[] input = {4, 0, 0, 0, 0x4a, 0x61, 0x76, 0x61, 0, 8, 0, 0, 0}; + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, input); + buffer.position(4); + try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + + // when / then + stream.skipCString(); + + assertEquals(9, stream.getPosition()); + assertEquals(8, stream.readInt32()); + } + } + + + private static ByteBuf allocateAndWriteToBuffer(final BufferProvider bufferProvider, final byte[] input) { + ByteBuf buffer = bufferProvider.getBuffer(input.length); + buffer.put(input, 0, input.length); + buffer.flip(); + return buffer; + } + + + public static byte[] mergeArrays(byte[]... arrays) throws IOException { + int size = 0; + for (byte[] array : arrays) { + size += array.length; + } + ByteArrayOutputStream baos = new ByteArrayOutputStream(size); + for (byte[] array : arrays) { + baos.write(array); + } + return baos.toByteArray(); + } + + private static byte[] getEncodedString(final String str) { + byte[] encoding = str.getBytes(StandardCharsets.UTF_8); + int littleEndianLength = reverseBytes(encoding.length + "\u0000".length()); + byte[] length = Ints.toByteArray(littleEndianLength); + + byte[] combined = new byte[encoding.length + length.length + 1]; + System.arraycopy(length, 0, combined, 0, length.length); + System.arraycopy(encoding, 0, combined, length.length, encoding.length); + return combined; + } + + private static byte[] getEncodedCString(final String str) { + byte[] encoding = str.getBytes(StandardCharsets.UTF_8); + byte[] combined = new byte[encoding.length + 1]; + System.arraycopy(encoding, 0, combined, 0, encoding.length); + return combined; + } +} \ No newline at end of file From 04965cd46b3ce091e9309f7175b3a7f2a984320d Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 2 Apr 2025 23:28:43 -0700 Subject: [PATCH 02/18] Add scratch buffer read optimization. JAVA-5814 --- .../main/org/bson/internal/PlatformUtil.java | 53 ++++++++++++++++ .../main/org/bson/io/ByteBufferBsonInput.java | 63 ++++++++++++------- .../org/bson/internal/PlatformUtilTest.java | 56 +++++++++++++++++ 3 files changed, 150 insertions(+), 22 deletions(-) create mode 100644 bson/src/main/org/bson/internal/PlatformUtil.java create mode 100644 bson/src/test/unit/org/bson/internal/PlatformUtilTest.java diff --git a/bson/src/main/org/bson/internal/PlatformUtil.java b/bson/src/main/org/bson/internal/PlatformUtil.java new file mode 100644 index 00000000000..52b1b6183c8 --- /dev/null +++ b/bson/src/main/org/bson/internal/PlatformUtil.java @@ -0,0 +1,53 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.bson.internal; + +/** + * Utility class for platform-specific operations. + * This class is not part of the public API and may be removed or changed at any time. + */ +public class PlatformUtil { + + private PlatformUtil() { + //NOOP + } + + // These architectures support unaligned memory access. + // While others might as well, it's safer to assume they don't. + private static final String[] ARCHITECTURES_ALLOWING_UNALIGNED_ACCESS = { + "x86", + "amd64", + "i386", + "x86_64", + "arm64", + "aarch64"}; + + public static boolean isUnalignedAccessAllowed() { + try { + String processArch = System.getProperty("os.arch"); + for (String supportedArch : ARCHITECTURES_ALLOWING_UNALIGNED_ACCESS) { + if (supportedArch.equals(processArch)) { + return true; + } + } + return false; + } catch (Exception e) { + // Ignore security exception and proceed with default value + return false; + } + } +} diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 1f553d02f77..9e552228f54 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -24,6 +24,7 @@ import java.nio.charset.StandardCharsets; import static java.lang.String.format; +import static org.bson.internal.PlatformUtil.isUnalignedAccessAllowed; /** * An implementation of {@code BsonInput} that is backed by a {@code ByteBuf}. @@ -33,6 +34,10 @@ public class ByteBufferBsonInput implements BsonInput { private static final String[] ONE_BYTE_ASCII_STRINGS = new String[Byte.MAX_VALUE + 1]; + private static final boolean UNALIGNED_ACCESS_SUPPORTED = isUnalignedAccessAllowed(); + private int scratchBufferSize = 0; + private byte[] scratchBuffer; + static { for (int b = 0; b < ONE_BYTE_ASCII_STRINGS.length; b++) { @@ -131,8 +136,8 @@ public String readCString() { return readString(size); } - private String readString(final int size) { - if (size == 2) { + private String readString(final int stringSize) { + if (stringSize == 2) { byte asciiByte = buffer.get(); // if only one byte in the string, it must be ascii. byte nullByte = buffer.get(); // read null terminator if (nullByte != 0) { @@ -146,20 +151,29 @@ private String readString(final int size) { if (buffer.hasArray()) { int position = buffer.position(); int arrayOffset = buffer.arrayOffset(); - buffer.position(position + size - 1); + buffer.position(position + stringSize - 1); byte nullByte = buffer.get(); if (nullByte != 0) { throw new BsonSerializationException("Found a BSON string that is not null-terminated"); } - return new String(buffer.array(), arrayOffset + position, size - 1, StandardCharsets.UTF_8); + return new String(buffer.array(), arrayOffset + position, stringSize - 1, StandardCharsets.UTF_8); } - byte[] bytes = new byte[size]; - buffer.get(bytes); - if (bytes[size - 1] != 0) { - throw new BsonSerializationException("BSON string not null-terminated"); + if (stringSize <= scratchBufferSize) { + buffer.get(scratchBuffer, 0, stringSize); + if (scratchBuffer[stringSize - 1] != 0) { + throw new BsonSerializationException("BSON string not null-terminated"); + } + return new String(scratchBuffer, 0, stringSize - 1, StandardCharsets.UTF_8); + } else { + scratchBufferSize = stringSize + (stringSize >> 1); //1.5 times the size + scratchBuffer = new byte[scratchBufferSize]; + buffer.get(scratchBuffer, 0, stringSize); + if (scratchBuffer[stringSize - 1] != 0) { + throw new BsonSerializationException("BSON string not null-terminated"); + } + return new String(scratchBuffer, 0, stringSize - 1, StandardCharsets.UTF_8); } - return new String(bytes, 0, size - 1, StandardCharsets.UTF_8); } } @@ -173,21 +187,25 @@ public void skipCString() { public int computeCStringLength(int prevPos) { ensureOpen(); - //TODO should we fall back to byte-by-byte search on JDK 8? ByteBuffer does not use Unsafe internally - int pos = prevPos; + int pos = buffer.position(); int limit = buffer.limit(); - while (limit - pos >= Long.BYTES) { - long word = buffer.getLong(pos); - long mask = word - 0x0101010101010101L; - mask &= ~word; - mask &= 0x8080808080808080L; - if (mask != 0) { - // first null terminator found in the Little Endian long - int offset = Long.numberOfTrailingZeros(mask) >>> 3; - // Found the null at pos + offset; reset buffer's position. - return (pos - prevPos) + offset + 1; + + if (UNALIGNED_ACCESS_SUPPORTED) { + int chunks = (limit - pos) >>> 3; + // Process 8 bytes at a time. + for (int i = 0; i < chunks; i++) { + long word = buffer.getLong(pos); + long mask = word - 0x0101010101010101L; + mask &= ~word; + mask &= 0x8080808080808080L; + if (mask != 0) { + // first null terminator found in the Little Endian long + int offset = Long.numberOfTrailingZeros(mask) >>> 3; + // Found the null at pos + offset; reset buffer's position. + return pos + offset; + } + pos += 8; } - pos += 8; } // Process remaining bytes one-by-one. @@ -196,6 +214,7 @@ public int computeCStringLength(int prevPos) { return (pos - prevPos); } } + buffer.position(pos); throw new BsonSerializationException("Found a BSON string that is not null-terminated"); } diff --git a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java new file mode 100644 index 00000000000..e0385fc38a1 --- /dev/null +++ b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java @@ -0,0 +1,56 @@ +package org.bson.internal; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +class PlatformUtilTest { + + @ParameterizedTest + @ValueSource(strings = {"arm", "arm64", "aarch64", "ppc", "ppc64", "sparc", "mips"}) + @DisplayName("Should not allow unaligned access for unsupported architectures") + void shouldNotAllowUnalignedAccessForUnsupportedArchitecture(final String architecture) { + withSystemProperty("os.arch", architecture, () -> { + boolean result = PlatformUtil.isUnalignedAccessAllowed(); + assertFalse(result); + }); + } + + @Test + @DisplayName("Should not allow unaligned access when system property is undefined") + void shouldNotAllowUnalignedAccessWhenSystemPropertyIsUndefined() { + withSystemProperty("os.arch", null, () -> { + boolean result = PlatformUtil.isUnalignedAccessAllowed(); + assertFalse(result); + }); + } + + @ParameterizedTest + @ValueSource(strings = {"x86", "amd64", "i386", "x86_64"}) + @DisplayName("Should allow unaligned access for supported architectures") + void shouldAllowUnalignedAccess(final String architecture) { + withSystemProperty("os.arch", architecture, () -> { + boolean result = PlatformUtil.isUnalignedAccessAllowed(); + assertTrue(result); + }); + } + + public static void withSystemProperty(final String name, final String value, final Runnable testCode) { + String original = System.getProperty(name); + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + try { + testCode.run(); + } finally { + System.setProperty(name, original); + } + } + +} \ No newline at end of file From 955b8c594da03500f566a8bd7acb1a5743cb51d0 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 2 Apr 2025 23:30:52 -0700 Subject: [PATCH 03/18] Remove TODO. JAVA-5814 --- .../main/com/mongodb/internal/connection/ResponseBuffers.java | 1 - 1 file changed, 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java b/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java index 7dd897ab556..d549888096a 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java @@ -59,7 +59,6 @@ T getResponseDocument(final int messageId, final Decode * @return a read-only buffer containing the response body */ public ByteBuf getBodyByteBuffer() { - //TODO any side-effects to make it read-only? return bodyByteBuffer; } From dda00e3ffc8c959580c32041c682e8669fac133a Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 2 Apr 2025 23:45:21 -0700 Subject: [PATCH 04/18] Address static check warnings. JAVA-5814 --- .../main/org/bson/internal/PlatformUtil.java | 3 +- .../main/org/bson/io/ByteBufferBsonInput.java | 2 +- .../org/bson/internal/PlatformUtilTest.java | 21 +++++++-- .../connection/ByteBufferBsonInputeTest.java | 44 +++++++++++++------ 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/bson/src/main/org/bson/internal/PlatformUtil.java b/bson/src/main/org/bson/internal/PlatformUtil.java index 52b1b6183c8..e6897013318 100644 --- a/bson/src/main/org/bson/internal/PlatformUtil.java +++ b/bson/src/main/org/bson/internal/PlatformUtil.java @@ -20,7 +20,7 @@ * Utility class for platform-specific operations. * This class is not part of the public API and may be removed or changed at any time. */ -public class PlatformUtil { +public final class PlatformUtil { private PlatformUtil() { //NOOP @@ -51,3 +51,4 @@ public static boolean isUnalignedAccessAllowed() { } } } + diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 9e552228f54..9d534c32c7a 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -185,7 +185,7 @@ public void skipCString() { buffer.position(pos + length); } - public int computeCStringLength(int prevPos) { + public int computeCStringLength(final int prevPos) { ensureOpen(); int pos = buffer.position(); int limit = buffer.limit(); diff --git a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java index e0385fc38a1..0718497dc29 100644 --- a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java +++ b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java @@ -1,3 +1,19 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.bson.internal; import org.junit.jupiter.api.DisplayName; @@ -30,7 +46,7 @@ void shouldNotAllowUnalignedAccessWhenSystemPropertyIsUndefined() { } @ParameterizedTest - @ValueSource(strings = {"x86", "amd64", "i386", "x86_64"}) + @ValueSource(strings = {"x86", "amd64", "i386", "x86_64", "arm64", "aarch64"}) @DisplayName("Should allow unaligned access for supported architectures") void shouldAllowUnalignedAccess(final String architecture) { withSystemProperty("os.arch", architecture, () -> { @@ -52,5 +68,4 @@ public static void withSystemProperty(final String name, final String value, fin System.setProperty(name, original); } } - -} \ No newline at end of file +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java index 6226052e71e..adbc8e0ebbd 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java @@ -1,3 +1,19 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.mongodb.internal.connection; import com.google.common.primitives.Ints; @@ -132,8 +148,8 @@ void shouldReadStringUptoBufferLimit(final BufferProvider bufferProvider) { // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + - String.valueOf(Character.toChars(codePoint)); + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); byte[] expectedStringEncoding = getEncodedString(str); ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); @@ -154,8 +170,8 @@ void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) t // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + - String.valueOf(Character.toChars(codePoint)); + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); byte[] expectedStringEncoding = getEncodedString(str); byte[] bufferBytes = mergeArrays( expectedStringEncoding, @@ -181,8 +197,8 @@ void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IO // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + - String.valueOf(Character.toChars(codePoint)); + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); byte[] expectedStringEncoding = getEncodedString(str); byte[] bufferBytes = mergeArrays( @@ -211,8 +227,8 @@ void shouldReadCStringUptoBufferLimit(final BufferProvider bufferProvider) { // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + - String.valueOf(Character.toChars(codePoint)); + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); byte[] expectedStringEncoding = getEncodedCString(str); ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); @@ -233,8 +249,8 @@ void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider) // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + - String.valueOf(Character.toChars(codePoint)); + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); byte[] expectedStringEncoding = getEncodedCString(str); byte[] bufferBytes = mergeArrays( expectedStringEncoding, @@ -261,8 +277,8 @@ void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { //given - String str = join("", nCopies(offset, "b")) + - String.valueOf(Character.toChars(codePoint)); + String str = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); byte[] expectedStringEncoding = getEncodedCString(str); byte[] bufferBytes = mergeArrays( @@ -443,7 +459,7 @@ private static ByteBuf allocateAndWriteToBuffer(final BufferProvider bufferProvi } - public static byte[] mergeArrays(byte[]... arrays) throws IOException { + public static byte[] mergeArrays(final byte[]... arrays) throws IOException { int size = 0; for (byte[] array : arrays) { size += array.length; @@ -472,4 +488,4 @@ private static byte[] getEncodedCString(final String str) { System.arraycopy(encoding, 0, combined, 0, encoding.length); return combined; } -} \ No newline at end of file +} From 579e2b1ba1e0a0c499213b6c5c442b3b4ffbd4ba Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 2 Apr 2025 23:51:16 -0700 Subject: [PATCH 05/18] Fix index calculation. JAVA-5814 --- bson/src/main/org/bson/io/ByteBufferBsonInput.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 9d534c32c7a..1575e076193 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -202,7 +202,7 @@ public int computeCStringLength(final int prevPos) { // first null terminator found in the Little Endian long int offset = Long.numberOfTrailingZeros(mask) >>> 3; // Found the null at pos + offset; reset buffer's position. - return pos + offset; + return (pos - prevPos) + offset + 1; } pos += 8; } From 89148132c047c16a57e77c92f740d7022ff1045b Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Thu, 3 Apr 2025 16:59:50 -0700 Subject: [PATCH 06/18] Allocate direct buffer in tests. JAVA-5814 --- ...yteBufferBsonInputeTest.java => ByteBufferBsonInputTest.java} | 1 + 1 file changed, 1 insertion(+) rename driver-core/src/test/unit/com/mongodb/internal/connection/{ByteBufferBsonInputeTest.java => ByteBufferBsonInputTest.java} (99%) diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java similarity index 99% rename from driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java rename to driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index adbc8e0ebbd..38c41c062fa 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputeTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -64,6 +64,7 @@ static Stream bufferProviders() { new PowerOfTwoBufferPool(), size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 5], 2, size).slice()), //different array offsets size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 4], 3, size).slice()), //different array offsets + size -> new ByteBufNIO(ByteBuffer.allocateDirect(size)), size -> new ByteBufNIO(ByteBuffer.allocate(size)) { @Override public boolean hasArray() { From 9422cf93dfc6de86126b5bb78b7fd8b36029b321 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Fri, 4 Apr 2025 15:21:08 -0700 Subject: [PATCH 07/18] Remove redundant branching. Add tests. JAVA-5842 --- .../main/org/bson/io/ByteBufferBsonInput.java | 31 +- .../internal/connection/ResponseBuffers.java | 6 +- .../connection/ByteBufferBsonInputTest.java | 316 ++++++++++++++---- 3 files changed, 268 insertions(+), 85 deletions(-) diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 1575e076193..3fc458fe6e4 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -151,29 +151,24 @@ private String readString(final int stringSize) { if (buffer.hasArray()) { int position = buffer.position(); int arrayOffset = buffer.arrayOffset(); - buffer.position(position + stringSize - 1); - byte nullByte = buffer.get(); - if (nullByte != 0) { - throw new BsonSerializationException("Found a BSON string that is not null-terminated"); - } - return new String(buffer.array(), arrayOffset + position, stringSize - 1, StandardCharsets.UTF_8); - } + int newPosition = position + stringSize; + buffer.position(newPosition); - if (stringSize <= scratchBufferSize) { - buffer.get(scratchBuffer, 0, stringSize); - if (scratchBuffer[stringSize - 1] != 0) { - throw new BsonSerializationException("BSON string not null-terminated"); + byte[] array = buffer.array(); + if (array[arrayOffset + newPosition - 1] != 0) { + throw new BsonSerializationException("Found a BSON string that is not null-terminated"); } - return new String(scratchBuffer, 0, stringSize - 1, StandardCharsets.UTF_8); - } else { + return new String(array, arrayOffset + position, stringSize - 1, StandardCharsets.UTF_8); + } else if (stringSize > scratchBufferSize) { scratchBufferSize = stringSize + (stringSize >> 1); //1.5 times the size scratchBuffer = new byte[scratchBufferSize]; - buffer.get(scratchBuffer, 0, stringSize); - if (scratchBuffer[stringSize - 1] != 0) { - throw new BsonSerializationException("BSON string not null-terminated"); - } - return new String(scratchBuffer, 0, stringSize - 1, StandardCharsets.UTF_8); } + + buffer.get(scratchBuffer, 0, stringSize); + if (scratchBuffer[stringSize - 1] != 0) { + throw new BsonSerializationException("BSON string not null-terminated"); + } + return new String(scratchBuffer, 0, stringSize - 1, StandardCharsets.UTF_8); } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java b/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java index d549888096a..6774b4a50af 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java @@ -53,10 +53,12 @@ T getResponseDocument(final int messageId, final Decode } /** - * Returns a read-only buffer containing the response body. Care should be taken to not use the returned buffer after this instance has + * Returns a buffer containing the response body. Care should be taken to not use the returned buffer after this instance has * been closed. * - * @return a read-only buffer containing the response body + * NOTE: do not modify this buffer, it is being made writable for performance reasons to avoid redundant copying. + * + * @return a buffer containing the response body */ public ByteBuf getBodyByteBuffer() { return bodyByteBuffer; diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index 38c41c062fa..4ab5fc3ae82 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -122,7 +122,7 @@ void shouldReadInvalidOneByteString(final BufferProvider bufferProvider) { ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{2, 0, 0, 0, (byte) 0xFF, 0}); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then + // when & then String result = bufferInput.readString(); assertEquals("\uFFFD", result); assertEquals(6, bufferInput.getPosition()); @@ -135,7 +135,7 @@ void shouldReadInvalidOneByteCString(final BufferProvider bufferProvider) { ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{-0x01, 0}); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then + // when & then String result = bufferInput.readCString(); assertEquals("\uFFFD", result); assertEquals(2, bufferInput.getPosition()); @@ -149,16 +149,16 @@ void shouldReadStringUptoBufferLimit(final BufferProvider bufferProvider) { // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + String expectedString = join("", nCopies(offset, "b")) + String.valueOf(Character.toChars(codePoint)); - byte[] expectedStringEncoding = getEncodedString(str); + byte[] expectedStringEncoding = getExpectedEncodedString(expectedString); ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - String result = bufferInput.readString(); - assertEquals(str, result); + // when & then + String actualString = bufferInput.readString(); + assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } } @@ -171,9 +171,9 @@ void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) t // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + String expectedString = join("", nCopies(offset, "b")) + String.valueOf(Character.toChars(codePoint)); - byte[] expectedStringEncoding = getEncodedString(str); + byte[] expectedStringEncoding = getExpectedEncodedString(expectedString); byte[] bufferBytes = mergeArrays( expectedStringEncoding, new byte[]{1, 2, 3} @@ -183,25 +183,211 @@ void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) t try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - String result = bufferInput.readString(); - assertEquals(str, result); + // when & then + String actualString = bufferInput.readString(); + assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } } } } + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadMultipleStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String expectedString1 = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + String expectedString2 = join("", nCopies(offset, "a")) + + String.valueOf(Character.toChars(codePoint)); + + byte[] expectedStringEncoding1 = getExpectedEncodedString(expectedString1); + byte[] expectedStringEncoding2 = getExpectedEncodedString(expectedString2); + int expectedInteger = 12412; + byte[] bufferBytes = mergeArrays( + new byte[]{1, 2, 3}, + expectedStringEncoding1, + Ints.toByteArray(reverseBytes(expectedInteger)), + expectedStringEncoding2, + new byte[]{1, 2, 3, 4} + ); + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + buffer.position(3); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when & then + String actualString1 = bufferInput.readString(); + assertEquals( + expectedString1, + actualString1); + assertEquals( + 3 + expectedStringEncoding1.length, + bufferInput.getPosition()); + + assertEquals(expectedInteger, bufferInput.readInt32()); + + String actualString2 = bufferInput.readString(); + assertEquals( + expectedString2, + actualString2); + assertEquals( + 3 + expectedStringEncoding1.length + expectedStringEncoding2.length + Integer.BYTES, + bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadConsecutiveMultipleStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String expectedString1 = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + String expectedString2 = join("", nCopies(offset, "a")) + + String.valueOf(Character.toChars(codePoint)); + + byte[] expectedStringEncoding1 = getExpectedEncodedString(expectedString1); + byte[] expectedStringEncoding2 = getExpectedEncodedString(expectedString2); + byte[] bufferBytes = mergeArrays( + new byte[]{1, 2, 3}, + expectedStringEncoding1, + expectedStringEncoding2, + new byte[]{1, 2, 3, 4} + ); + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + buffer.position(3); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when & then + String actualString1 = bufferInput.readString(); + assertEquals( + expectedString1, + actualString1); + assertEquals( + 3 + expectedStringEncoding1.length, + bufferInput.getPosition()); + + String actualString2 = bufferInput.readString(); + assertEquals( + expectedString2, + actualString2); + assertEquals( + 3 + expectedStringEncoding1.length + expectedStringEncoding2.length, + bufferInput.getPosition()); + } + } + + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldConsecutiveReadMultipleCStringsWithinInBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String expectedString1 = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + String expectedString2 = join("", nCopies(offset, "a")) + + String.valueOf(Character.toChars(codePoint)); + + byte[] expectedStringEncoding1 = getExpectedEncodedCString(expectedString1); + byte[] expectedStringEncoding2 = getExpectedEncodedCString(expectedString2); + byte[] bufferBytes = mergeArrays( + new byte[]{1, 2, 3}, + expectedStringEncoding1, + expectedStringEncoding2, + new byte[]{1, 2, 3, 4} + ); + + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + buffer.position(3); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when & then + String actualString1 = bufferInput.readCString(); + assertEquals( + expectedString1, + actualString1); + assertEquals( + 3 + expectedStringEncoding1.length, + bufferInput.getPosition()); + + String actualString2 = bufferInput.readCString(); + assertEquals( + expectedString2, + actualString2); + assertEquals( + 3 + expectedStringEncoding1.length + expectedStringEncoding2.length, + bufferInput.getPosition()); + } + } + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldReadMultipleCStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException { + // given + for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { + for (int offset = 0; offset < 18; offset++) { + String expectedString1 = join("", nCopies(offset, "b")) + + String.valueOf(Character.toChars(codePoint)); + String expectedString2 = join("", nCopies(offset, "a")) + + String.valueOf(Character.toChars(codePoint)); + + byte[] expectedStringEncoding1 = getExpectedEncodedCString(expectedString1); + byte[] expectedStringEncoding2 = getExpectedEncodedCString(expectedString2); + int expectedInteger = 12412; + byte[] bufferBytes = mergeArrays( + new byte[]{1, 2, 3}, + expectedStringEncoding1, + Ints.toByteArray(reverseBytes(expectedInteger)), + expectedStringEncoding2, + new byte[]{1, 2, 3, 4} + ); + ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, bufferBytes); + buffer.position(3); + + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { + // when & then + String actualString1 = bufferInput.readCString(); + assertEquals( + expectedString1, + actualString1); + assertEquals( + 3 + expectedStringEncoding1.length, + bufferInput.getPosition()); + + assertEquals(expectedInteger, bufferInput.readInt32()); + + String actualString2 = bufferInput.readCString(); + assertEquals( + expectedString2, + actualString2); + assertEquals( + 3 + expectedStringEncoding1.length + expectedStringEncoding2.length + Integer.BYTES, + bufferInput.getPosition()); + } + } + } + } + @ParameterizedTest @MethodSource("bufferProviders") void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IOException { // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + String expectedString = join("", nCopies(offset, "b")) + String.valueOf(Character.toChars(codePoint)); - byte[] expectedStringEncoding = getEncodedString(str); + byte[] expectedStringEncoding = getExpectedEncodedString(expectedString); byte[] bufferBytes = mergeArrays( new byte[]{1, 2, 3}, expectedStringEncoding, @@ -213,9 +399,9 @@ void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IO try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - String result = bufferInput.readString(); - assertEquals(str, result); + // when & then + String actualString = bufferInput.readString(); + assertEquals(expectedString, actualString); assertEquals(3 + expectedStringEncoding.length, bufferInput.getPosition()); } } @@ -228,16 +414,16 @@ void shouldReadCStringUptoBufferLimit(final BufferProvider bufferProvider) { // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + String expectedString = join("", nCopies(offset, "b")) + String.valueOf(Character.toChars(codePoint)); - byte[] expectedStringEncoding = getEncodedCString(str); + byte[] expectedStringEncoding = getExpectedEncodedCString(expectedString); ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - String result = bufferInput.readCString(); - assertEquals(str, result); + // when & then + String actualString = bufferInput.readCString(); + assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } } @@ -250,9 +436,9 @@ void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider) // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { - String str = join("", nCopies(offset, "b")) + String expectedString = join("", nCopies(offset, "b")) + String.valueOf(Character.toChars(codePoint)); - byte[] expectedStringEncoding = getEncodedCString(str); + byte[] expectedStringEncoding = getExpectedEncodedCString(expectedString); byte[] bufferBytes = mergeArrays( expectedStringEncoding, new byte[]{1, 2, 3} @@ -262,9 +448,9 @@ void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider) try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - String result = bufferInput.readCString(); - assertEquals(str, result); + // when & then + String actualString = bufferInput.readCString(); + assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } } @@ -278,10 +464,10 @@ void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { //given - String str = join("", nCopies(offset, "b")) + String expectedString = join("", nCopies(offset, "b")) + String.valueOf(Character.toChars(codePoint)); - byte[] expectedStringEncoding = getEncodedCString(str); + byte[] expectedStringEncoding = getExpectedEncodedCString(expectedString); byte[] bufferBytes = mergeArrays( new byte[]{1, 2, 3}, expectedStringEncoding, @@ -292,9 +478,9 @@ void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws buffer.position(3); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - String result = bufferInput.readCString(); - assertEquals(str, result); + // when & then + String actualString = bufferInput.readCString(); + assertEquals(expectedString, actualString); assertEquals(3 + expectedStringEncoding.length, bufferInput.getPosition()); } } @@ -306,10 +492,10 @@ void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws void shouldThrowIfCStringIsNotNullTerminatedSkip(final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{(byte) 0xe0, (byte) 0xa4, (byte) 0x80}); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput expectedString = new ByteBufferBsonInput(buffer)) { - // when / then - assertThrows(BsonSerializationException.class, stream::skipCString); + // when & then + assertThrows(BsonSerializationException.class, expectedString::skipCString); } } @@ -333,10 +519,10 @@ public static Stream nonNullTerminatedStringsWithBuffers() { void shouldThrowIfStringIsNotNullTerminated(final byte[] nonNullTerminatedString, final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, nonNullTerminatedString); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput expectedStringeam = new ByteBufferBsonInput(buffer)) { - // when / then - assertThrows(BsonSerializationException.class, stream::readString); + // when & then + assertThrows(BsonSerializationException.class, expectedStringeam::readString); } } @@ -361,7 +547,7 @@ void shouldThrowIfCStringIsNotNullTerminated(final byte[] nonNullTerminatedCStri ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, nonNullTerminatedCString); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then + // when & then assertThrows(BsonSerializationException.class, bufferInput::readCString); } } @@ -372,10 +558,10 @@ void shouldThrowIfCStringIsNotNullTerminated(final byte[] nonNullTerminatedCStri void shouldThrowIfOneByteStringIsNotNullTerminated(final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{2, 0, 0, 0, 1}); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - assertThrows(BsonSerializationException.class, stream::readString); + // when & then + assertThrows(BsonSerializationException.class, bufferInput::readString); } } @@ -384,10 +570,10 @@ void shouldThrowIfOneByteStringIsNotNullTerminated(final BufferProvider bufferPr void shouldThrowIfOneByteCStringIsNotNullTerminated(final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{1}); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - assertThrows(BsonSerializationException.class, stream::readCString); + // when & then + assertThrows(BsonSerializationException.class, bufferInput::readCString); } } @@ -396,10 +582,10 @@ void shouldThrowIfOneByteCStringIsNotNullTerminated(final BufferProvider bufferP void shouldThrowIfLengthOfBsonStringIsNotPositive(final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{-1, -1, -1, -1, 41, 42, 43, 0}); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - assertThrows(BsonSerializationException.class, stream::readString); + // when & then + assertThrows(BsonSerializationException.class, bufferInput::readString); } } @@ -424,13 +610,13 @@ public static Stream shouldSkipCStringWhenMultipleNullTerminationPres void shouldSkipCStringWhenMultipleNullTerminationPresent(final byte[] cStringBytes, final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, cStringBytes); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - stream.skipCString(); + // when & then + bufferInput.skipCString(); - assertEquals(cStringBytes.length - Integer.BYTES, stream.getPosition()); - assertEquals(8, stream.readInt32()); + assertEquals(cStringBytes.length - Integer.BYTES, bufferInput.getPosition()); + assertEquals(8, bufferInput.readInt32()); } } @@ -441,13 +627,13 @@ void shouldReadSkipCStringWhenMultipleNullTerminationPresentWithinBuffer(final B byte[] input = {4, 0, 0, 0, 0x4a, 0x61, 0x76, 0x61, 0, 8, 0, 0, 0}; ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, input); buffer.position(4); - try (ByteBufferBsonInput stream = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when / then - stream.skipCString(); + // when & then + bufferInput.skipCString(); - assertEquals(9, stream.getPosition()); - assertEquals(8, stream.readInt32()); + assertEquals(9, bufferInput.getPosition()); + assertEquals(8, bufferInput.readInt32()); } } @@ -472,19 +658,19 @@ public static byte[] mergeArrays(final byte[]... arrays) throws IOException { return baos.toByteArray(); } - private static byte[] getEncodedString(final String str) { - byte[] encoding = str.getBytes(StandardCharsets.UTF_8); - int littleEndianLength = reverseBytes(encoding.length + "\u0000".length()); + private static byte[] getExpectedEncodedString(final String expectedString) { + byte[] expectedEncoding = expectedString.getBytes(StandardCharsets.UTF_8);//baseline + int littleEndianLength = reverseBytes(expectedEncoding.length + "\u0000".length()); byte[] length = Ints.toByteArray(littleEndianLength); - byte[] combined = new byte[encoding.length + length.length + 1]; + byte[] combined = new byte[expectedEncoding.length + length.length + 1]; System.arraycopy(length, 0, combined, 0, length.length); - System.arraycopy(encoding, 0, combined, length.length, encoding.length); + System.arraycopy(expectedEncoding, 0, combined, length.length, expectedEncoding.length); return combined; } - private static byte[] getEncodedCString(final String str) { - byte[] encoding = str.getBytes(StandardCharsets.UTF_8); + private static byte[] getExpectedEncodedCString(final String expectedString) { + byte[] encoding = expectedString.getBytes(StandardCharsets.UTF_8); byte[] combined = new byte[encoding.length + 1]; System.arraycopy(encoding, 0, combined, 0, encoding.length); return combined; From 95e890f32caee2ed7be7a2a3ce9a38c0366c9700 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Fri, 4 Apr 2025 15:26:21 -0700 Subject: [PATCH 08/18] Split when & then comments. JAVA-5842 --- .../connection/ByteBufferBsonInputTest.java | 71 +++++++++++++++---- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index 4ab5fc3ae82..c01d18e83d6 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -122,8 +122,10 @@ void shouldReadInvalidOneByteString(final BufferProvider bufferProvider) { ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{2, 0, 0, 0, (byte) 0xFF, 0}); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String result = bufferInput.readString(); + + // then assertEquals("\uFFFD", result); assertEquals(6, bufferInput.getPosition()); } @@ -135,8 +137,10 @@ void shouldReadInvalidOneByteCString(final BufferProvider bufferProvider) { ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{-0x01, 0}); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String result = bufferInput.readCString(); + + // then assertEquals("\uFFFD", result); assertEquals(2, bufferInput.getPosition()); } @@ -156,8 +160,10 @@ void shouldReadStringUptoBufferLimit(final BufferProvider bufferProvider) { ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, expectedStringEncoding); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString = bufferInput.readString(); + + // then assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } @@ -183,8 +189,10 @@ void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) t try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString = bufferInput.readString(); + + // then assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } @@ -217,8 +225,10 @@ void shouldReadMultipleStringsWithinBuffer(final BufferProvider bufferProvider) buffer.position(3); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString1 = bufferInput.readString(); + + // then assertEquals( expectedString1, actualString1); @@ -226,8 +236,10 @@ void shouldReadMultipleStringsWithinBuffer(final BufferProvider bufferProvider) 3 + expectedStringEncoding1.length, bufferInput.getPosition()); + // when assertEquals(expectedInteger, bufferInput.readInt32()); + // then String actualString2 = bufferInput.readString(); assertEquals( expectedString2, @@ -263,8 +275,10 @@ void shouldReadConsecutiveMultipleStringsWithinBuffer(final BufferProvider buffe buffer.position(3); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString1 = bufferInput.readString(); + + // then assertEquals( expectedString1, actualString1); @@ -272,7 +286,10 @@ void shouldReadConsecutiveMultipleStringsWithinBuffer(final BufferProvider buffe 3 + expectedStringEncoding1.length, bufferInput.getPosition()); + // when String actualString2 = bufferInput.readString(); + + // then assertEquals( expectedString2, actualString2); @@ -309,8 +326,10 @@ void shouldConsecutiveReadMultipleCStringsWithinInBuffer(final BufferProvider bu buffer.position(3); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString1 = bufferInput.readCString(); + + // then assertEquals( expectedString1, actualString1); @@ -318,7 +337,10 @@ void shouldConsecutiveReadMultipleCStringsWithinInBuffer(final BufferProvider bu 3 + expectedStringEncoding1.length, bufferInput.getPosition()); + // when String actualString2 = bufferInput.readCString(); + + // then assertEquals( expectedString2, actualString2); @@ -355,8 +377,10 @@ void shouldReadMultipleCStringsWithinBuffer(final BufferProvider bufferProvider) buffer.position(3); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString1 = bufferInput.readCString(); + + // then assertEquals( expectedString1, actualString1); @@ -364,9 +388,16 @@ void shouldReadMultipleCStringsWithinBuffer(final BufferProvider bufferProvider) 3 + expectedStringEncoding1.length, bufferInput.getPosition()); - assertEquals(expectedInteger, bufferInput.readInt32()); + // when + int actualInteger = bufferInput.readInt32(); + + // then + assertEquals(expectedInteger, actualInteger); + // when String actualString2 = bufferInput.readCString(); + + // then assertEquals( expectedString2, actualString2); @@ -399,8 +430,10 @@ void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IO try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString = bufferInput.readString(); + + // then assertEquals(expectedString, actualString); assertEquals(3 + expectedStringEncoding.length, bufferInput.getPosition()); } @@ -421,8 +454,10 @@ void shouldReadCStringUptoBufferLimit(final BufferProvider bufferProvider) { try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString = bufferInput.readCString(); + + // then assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } @@ -448,8 +483,10 @@ void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider) try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString = bufferInput.readCString(); + + // then assertEquals(expectedString, actualString); assertEquals(expectedStringEncoding.length, bufferInput.getPosition()); } @@ -478,8 +515,10 @@ void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws buffer.position(3); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when String actualString = bufferInput.readCString(); + + // then assertEquals(expectedString, actualString); assertEquals(3 + expectedStringEncoding.length, bufferInput.getPosition()); } @@ -612,9 +651,10 @@ void shouldSkipCStringWhenMultipleNullTerminationPresent(final byte[] cStringByt ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, cStringBytes); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when bufferInput.skipCString(); + //then assertEquals(cStringBytes.length - Integer.BYTES, bufferInput.getPosition()); assertEquals(8, bufferInput.readInt32()); } @@ -629,9 +669,10 @@ void shouldReadSkipCStringWhenMultipleNullTerminationPresentWithinBuffer(final B buffer.position(4); try (ByteBufferBsonInput bufferInput = new ByteBufferBsonInput(buffer)) { - // when & then + // when bufferInput.skipCString(); + // then assertEquals(9, bufferInput.getPosition()); assertEquals(8, bufferInput.readInt32()); } From 84e7728f16ab12fc9110f85578af36f6a1d0b9bd Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Fri, 4 Apr 2025 15:28:43 -0700 Subject: [PATCH 09/18] Rename tests. JAVA-5842 --- .../mongodb/internal/connection/ByteBufferBsonInputTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index c01d18e83d6..077e55696a3 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -304,7 +304,7 @@ void shouldReadConsecutiveMultipleStringsWithinBuffer(final BufferProvider buffe @ParameterizedTest @MethodSource("bufferProviders") - void shouldConsecutiveReadMultipleCStringsWithinInBuffer(final BufferProvider bufferProvider) throws IOException { + void shouldReadConsecutiveMultipleCStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException { // given for (Integer codePoint : ALL_CODE_POINTS_EXCLUDING_SURROGATES) { for (int offset = 0; offset < 18; offset++) { From a3d3f44d57635c50d3d1a1ac9d6dff206c8321f7 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Fri, 4 Apr 2025 19:33:58 -0700 Subject: [PATCH 10/18] Rename method. JAVA-5842 --- bson/src/main/org/bson/ByteBuf.java | 2 +- bson/src/main/org/bson/internal/PlatformUtil.java | 4 ++-- bson/src/main/org/bson/io/ByteBufferBsonInput.java | 2 +- .../com/mongodb/internal/connection/CompositeByteBuf.java | 2 +- .../com/mongodb/internal/connection/netty/NettyByteBuf.java | 2 +- .../mongodb/internal/connection/ByteBufferBsonInputTest.java | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bson/src/main/org/bson/ByteBuf.java b/bson/src/main/org/bson/ByteBuf.java index d0708b61894..169bb649ab9 100644 --- a/bson/src/main/org/bson/ByteBuf.java +++ b/bson/src/main/org/bson/ByteBuf.java @@ -144,7 +144,7 @@ public interface ByteBuf { * @return {@code true} if, and only if, this buffer is backed by an array and is not read-only * @since 5.5 */ - boolean hasArray(); + boolean isBackedByArray(); /** * Returns the offset of the first byte within the backing byte array of diff --git a/bson/src/main/org/bson/internal/PlatformUtil.java b/bson/src/main/org/bson/internal/PlatformUtil.java index e6897013318..6dc610ce6f7 100644 --- a/bson/src/main/org/bson/internal/PlatformUtil.java +++ b/bson/src/main/org/bson/internal/PlatformUtil.java @@ -23,7 +23,7 @@ public final class PlatformUtil { private PlatformUtil() { - //NOOP + //NOP } // These architectures support unaligned memory access. @@ -33,7 +33,7 @@ private PlatformUtil() { "amd64", "i386", "x86_64", - "arm64", + "arm64", // evergreen dbx-perf-distro uses ths architecture "aarch64"}; public static boolean isUnalignedAccessAllowed() { diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 3fc458fe6e4..77aa0cb1ddf 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -148,7 +148,7 @@ private String readString(final int stringSize) { } return ONE_BYTE_ASCII_STRINGS[asciiByte]; // this will throw if asciiByte is negative } else { - if (buffer.hasArray()) { + if (buffer.isBackedByArray()) { int position = buffer.position(); int arrayOffset = buffer.arrayOffset(); int newPosition = position + stringSize; diff --git a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java index ca2ef40cc31..0af3678337a 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java @@ -214,7 +214,7 @@ public byte[] array() { } @Override - public boolean hasArray() { + public boolean isBackedByArray() { return false; } diff --git a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java index d1e0f43462e..84e738c68f9 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java @@ -101,7 +101,7 @@ public byte[] array() { } @Override - public boolean hasArray() { + public boolean isBackedByArray() { return proxied.hasArray(); } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index 077e55696a3..3eba33d11fb 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -67,7 +67,7 @@ static Stream bufferProviders() { size -> new ByteBufNIO(ByteBuffer.allocateDirect(size)), size -> new ByteBufNIO(ByteBuffer.allocate(size)) { @Override - public boolean hasArray() { + public boolean isBackedByArray() { return false; } From bdfe6d2746586389082ed079931e33f7759842a3 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Mon, 7 Apr 2025 21:11:31 -0700 Subject: [PATCH 11/18] Fix static checks. --- bson/src/main/org/bson/ByteBufNIO.java | 2 +- bson/src/main/org/bson/internal/PlatformUtil.java | 6 ++---- .../internal/connection/ByteBufferBsonInputTest.java | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/bson/src/main/org/bson/ByteBufNIO.java b/bson/src/main/org/bson/ByteBufNIO.java index bb949ce0860..b2ab26db7f9 100644 --- a/bson/src/main/org/bson/ByteBufNIO.java +++ b/bson/src/main/org/bson/ByteBufNIO.java @@ -109,7 +109,7 @@ public byte[] array() { } @Override - public boolean hasArray() { + public boolean isBackedByArray() { return buf.hasArray(); } diff --git a/bson/src/main/org/bson/internal/PlatformUtil.java b/bson/src/main/org/bson/internal/PlatformUtil.java index 6dc610ce6f7..997ec691519 100644 --- a/bson/src/main/org/bson/internal/PlatformUtil.java +++ b/bson/src/main/org/bson/internal/PlatformUtil.java @@ -22,9 +22,7 @@ */ public final class PlatformUtil { - private PlatformUtil() { - //NOP - } + private PlatformUtil() {} // These architectures support unaligned memory access. // While others might as well, it's safer to assume they don't. @@ -33,7 +31,7 @@ private PlatformUtil() { "amd64", "i386", "x86_64", - "arm64", // evergreen dbx-perf-distro uses ths architecture + "arm64", // evergreen dbx-perf-distro uses this architecture "aarch64"}; public static boolean isUnalignedAccessAllowed() { diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index 3eba33d11fb..1a6e504eeaf 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -700,7 +700,7 @@ public static byte[] mergeArrays(final byte[]... arrays) throws IOException { } private static byte[] getExpectedEncodedString(final String expectedString) { - byte[] expectedEncoding = expectedString.getBytes(StandardCharsets.UTF_8);//baseline + byte[] expectedEncoding = expectedString.getBytes(StandardCharsets.UTF_8); int littleEndianLength = reverseBytes(expectedEncoding.length + "\u0000".length()); byte[] length = Ints.toByteArray(littleEndianLength); From 95ab04bc64250a8e45b52192501fde22cf2628d5 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 9 Apr 2025 13:04:49 -0700 Subject: [PATCH 12/18] Fix test. --- bson/src/test/unit/org/bson/internal/PlatformUtilTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java index 0718497dc29..99a1d2e3e56 100644 --- a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java +++ b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java @@ -27,7 +27,7 @@ class PlatformUtilTest { @ParameterizedTest - @ValueSource(strings = {"arm", "arm64", "aarch64", "ppc", "ppc64", "sparc", "mips"}) + @ValueSource(strings = {"arm", "ppc", "ppc64", "sparc", "mips"}) @DisplayName("Should not allow unaligned access for unsupported architectures") void shouldNotAllowUnalignedAccessForUnsupportedArchitecture(final String architecture) { withSystemProperty("os.arch", architecture, () -> { From 7199945351461fd5770f7747e8b49da50dbe77df Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Fri, 11 Apr 2025 22:07:25 -0700 Subject: [PATCH 13/18] Add comments. --- .../main/org/bson/io/ByteBufferBsonInput.java | 28 ++++++++++++------- .../connection/ByteBufferBsonInputTest.java | 8 +++--- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 77aa0cb1ddf..7f19b031db5 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -35,7 +35,11 @@ public class ByteBufferBsonInput implements BsonInput { private static final String[] ONE_BYTE_ASCII_STRINGS = new String[Byte.MAX_VALUE + 1]; private static final boolean UNALIGNED_ACCESS_SUPPORTED = isUnalignedAccessAllowed(); - private int scratchBufferSize = 0; + /* A dynamically sized scratch buffer, that is reused across BSON String reads: + * 1. Reduces garbage collection by avoiding new byte array creation. + * 2. Improves cache utilization through temporal locality. + * 3. Avoids JVM allocation and zeroing cost for new memory allocations. + */ private byte[] scratchBuffer; @@ -136,8 +140,8 @@ public String readCString() { return readString(size); } - private String readString(final int stringSize) { - if (stringSize == 2) { + private String readString(final int bsonStringSize) { + if (bsonStringSize == 2) { byte asciiByte = buffer.get(); // if only one byte in the string, it must be ascii. byte nullByte = buffer.get(); // read null terminator if (nullByte != 0) { @@ -151,24 +155,24 @@ private String readString(final int stringSize) { if (buffer.isBackedByArray()) { int position = buffer.position(); int arrayOffset = buffer.arrayOffset(); - int newPosition = position + stringSize; + int newPosition = position + bsonStringSize; buffer.position(newPosition); byte[] array = buffer.array(); if (array[arrayOffset + newPosition - 1] != 0) { throw new BsonSerializationException("Found a BSON string that is not null-terminated"); } - return new String(array, arrayOffset + position, stringSize - 1, StandardCharsets.UTF_8); - } else if (stringSize > scratchBufferSize) { - scratchBufferSize = stringSize + (stringSize >> 1); //1.5 times the size + return new String(array, arrayOffset + position, bsonStringSize - 1, StandardCharsets.UTF_8); + } else if (scratchBuffer == null || bsonStringSize > scratchBuffer.length) { + int scratchBufferSize = bsonStringSize + (bsonStringSize >>> 1); //1.5 times the size scratchBuffer = new byte[scratchBufferSize]; } - buffer.get(scratchBuffer, 0, stringSize); - if (scratchBuffer[stringSize - 1] != 0) { + buffer.get(scratchBuffer, 0, bsonStringSize); + if (scratchBuffer[bsonStringSize - 1] != 0) { throw new BsonSerializationException("BSON string not null-terminated"); } - return new String(scratchBuffer, 0, stringSize - 1, StandardCharsets.UTF_8); + return new String(scratchBuffer, 0, bsonStringSize - 1, StandardCharsets.UTF_8); } } @@ -180,6 +184,10 @@ public void skipCString() { buffer.position(pos + length); } + /* + This method uses the SWAR (SIMD Within A Register) technique when aligned access is supported. + SWAR finds a null terminator by processing 8 bytes at once. + */ public int computeCStringLength(final int prevPos) { ensureOpen(); int pos = buffer.position(); diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java index 1a6e504eeaf..0846f7a54f1 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java @@ -73,12 +73,12 @@ public boolean isBackedByArray() { @Override public byte[] array() { - return Assertions.fail("array() is called, when hasArray() returns false"); + return Assertions.fail("array() is called, when isBackedByArray() returns false"); } @Override public int arrayOffset() { - return Assertions.fail("arrayOffset() is called, when hasArray() returns false"); + return Assertions.fail("arrayOffset() is called, when isBackedByArray() returns false"); } } ); @@ -558,10 +558,10 @@ public static Stream nonNullTerminatedStringsWithBuffers() { void shouldThrowIfStringIsNotNullTerminated(final byte[] nonNullTerminatedString, final BufferProvider bufferProvider) { // given ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, nonNullTerminatedString); - try (ByteBufferBsonInput expectedStringeam = new ByteBufferBsonInput(buffer)) { + try (ByteBufferBsonInput expectedString = new ByteBufferBsonInput(buffer)) { // when & then - assertThrows(BsonSerializationException.class, expectedStringeam::readString); + assertThrows(BsonSerializationException.class, expectedString::readString); } } From 5792d35b8a287b7f0ec663c067b859e08a8cad5a Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Thu, 17 Apr 2025 16:46:38 -0700 Subject: [PATCH 14/18] Fix ByteBufferBsonOutput buffer caching logic. --- .../mongodb/internal/connection/ByteBufferBsonOutput.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 64e26ed83c5..50d7be5d2c8 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -176,6 +176,7 @@ private ByteBuf getCurrentByteBuffer() { if (currentByteBuffer == null) { currentByteBuffer = getByteBufferAtIndex(curBufferIndex); } + if (currentByteBuffer.hasRemaining()) { return currentByteBuffer; } @@ -185,11 +186,6 @@ private ByteBuf getCurrentByteBuffer() { return currentByteBuffer; } - private ByteBuf getNextByteBuffer() { - assertFalse(bufferList.get(curBufferIndex).hasRemaining()); - return getByteBufferAtIndex(++curBufferIndex); - } - private ByteBuf getByteBufferAtIndex(final int index) { if (bufferList.size() < index + 1) { ByteBuf buffer = bufferProvider.getBuffer(index >= (MAX_SHIFT - INITIAL_SHIFT) @@ -459,7 +455,7 @@ private int writeOnBuffers(final String str, if (c < 0x80) { if (remaining == 0) { - curBuffer = getNextByteBuffer(); + curBuffer = getCurrentByteBuffer(); curBufferPos = 0; curBufferLimit = curBuffer.limit(); } From 0084f4fb0cbc12c36a715a30cd98febdd340045d Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Tue, 22 Apr 2025 17:33:06 -0700 Subject: [PATCH 15/18] Revert SWAR optimization. --- .../main/org/bson/io/ByteBufferBsonInput.java | 25 ------- .../org/bson/internal/PlatformUtilTest.java | 71 ------------------- 2 files changed, 96 deletions(-) delete mode 100644 bson/src/test/unit/org/bson/internal/PlatformUtilTest.java diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 7f19b031db5..8a2e463b6ed 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -24,7 +24,6 @@ import java.nio.charset.StandardCharsets; import static java.lang.String.format; -import static org.bson.internal.PlatformUtil.isUnalignedAccessAllowed; /** * An implementation of {@code BsonInput} that is backed by a {@code ByteBuf}. @@ -34,7 +33,6 @@ public class ByteBufferBsonInput implements BsonInput { private static final String[] ONE_BYTE_ASCII_STRINGS = new String[Byte.MAX_VALUE + 1]; - private static final boolean UNALIGNED_ACCESS_SUPPORTED = isUnalignedAccessAllowed(); /* A dynamically sized scratch buffer, that is reused across BSON String reads: * 1. Reduces garbage collection by avoiding new byte array creation. * 2. Improves cache utilization through temporal locality. @@ -184,34 +182,11 @@ public void skipCString() { buffer.position(pos + length); } - /* - This method uses the SWAR (SIMD Within A Register) technique when aligned access is supported. - SWAR finds a null terminator by processing 8 bytes at once. - */ public int computeCStringLength(final int prevPos) { ensureOpen(); int pos = buffer.position(); int limit = buffer.limit(); - if (UNALIGNED_ACCESS_SUPPORTED) { - int chunks = (limit - pos) >>> 3; - // Process 8 bytes at a time. - for (int i = 0; i < chunks; i++) { - long word = buffer.getLong(pos); - long mask = word - 0x0101010101010101L; - mask &= ~word; - mask &= 0x8080808080808080L; - if (mask != 0) { - // first null terminator found in the Little Endian long - int offset = Long.numberOfTrailingZeros(mask) >>> 3; - // Found the null at pos + offset; reset buffer's position. - return (pos - prevPos) + offset + 1; - } - pos += 8; - } - } - - // Process remaining bytes one-by-one. while (pos < limit) { if (buffer.get(pos++) == 0) { return (pos - prevPos); diff --git a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java b/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java deleted file mode 100644 index 99a1d2e3e56..00000000000 --- a/bson/src/test/unit/org/bson/internal/PlatformUtilTest.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright 2008-present MongoDB, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.bson.internal; - -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -class PlatformUtilTest { - - @ParameterizedTest - @ValueSource(strings = {"arm", "ppc", "ppc64", "sparc", "mips"}) - @DisplayName("Should not allow unaligned access for unsupported architectures") - void shouldNotAllowUnalignedAccessForUnsupportedArchitecture(final String architecture) { - withSystemProperty("os.arch", architecture, () -> { - boolean result = PlatformUtil.isUnalignedAccessAllowed(); - assertFalse(result); - }); - } - - @Test - @DisplayName("Should not allow unaligned access when system property is undefined") - void shouldNotAllowUnalignedAccessWhenSystemPropertyIsUndefined() { - withSystemProperty("os.arch", null, () -> { - boolean result = PlatformUtil.isUnalignedAccessAllowed(); - assertFalse(result); - }); - } - - @ParameterizedTest - @ValueSource(strings = {"x86", "amd64", "i386", "x86_64", "arm64", "aarch64"}) - @DisplayName("Should allow unaligned access for supported architectures") - void shouldAllowUnalignedAccess(final String architecture) { - withSystemProperty("os.arch", architecture, () -> { - boolean result = PlatformUtil.isUnalignedAccessAllowed(); - assertTrue(result); - }); - } - - public static void withSystemProperty(final String name, final String value, final Runnable testCode) { - String original = System.getProperty(name); - if (value == null) { - System.clearProperty(name); - } else { - System.setProperty(name, value); - } - try { - testCode.run(); - } finally { - System.setProperty(name, original); - } - } -} From cf515555306aea6f4ceb5ce0b64939ff2a9ed9fb Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 23 Apr 2025 18:09:10 -0700 Subject: [PATCH 16/18] Remove PlatformUtil. --- .../main/org/bson/internal/PlatformUtil.java | 52 ------------------- 1 file changed, 52 deletions(-) delete mode 100644 bson/src/main/org/bson/internal/PlatformUtil.java diff --git a/bson/src/main/org/bson/internal/PlatformUtil.java b/bson/src/main/org/bson/internal/PlatformUtil.java deleted file mode 100644 index 997ec691519..00000000000 --- a/bson/src/main/org/bson/internal/PlatformUtil.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright 2008-present MongoDB, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.bson.internal; - -/** - * Utility class for platform-specific operations. - * This class is not part of the public API and may be removed or changed at any time. - */ -public final class PlatformUtil { - - private PlatformUtil() {} - - // These architectures support unaligned memory access. - // While others might as well, it's safer to assume they don't. - private static final String[] ARCHITECTURES_ALLOWING_UNALIGNED_ACCESS = { - "x86", - "amd64", - "i386", - "x86_64", - "arm64", // evergreen dbx-perf-distro uses this architecture - "aarch64"}; - - public static boolean isUnalignedAccessAllowed() { - try { - String processArch = System.getProperty("os.arch"); - for (String supportedArch : ARCHITECTURES_ALLOWING_UNALIGNED_ACCESS) { - if (supportedArch.equals(processArch)) { - return true; - } - } - return false; - } catch (Exception e) { - // Ignore security exception and proceed with default value - return false; - } - } -} - From 9c20c99618cbf20c99b0fa67c163481329acb32f Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 23 Apr 2025 18:26:19 -0700 Subject: [PATCH 17/18] Fix static checks. --- .../com/mongodb/internal/connection/ByteBufferBsonOutput.java | 1 - 1 file changed, 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 50d7be5d2c8..600145db48f 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.List; -import static com.mongodb.assertions.Assertions.assertFalse; import static com.mongodb.assertions.Assertions.assertTrue; import static com.mongodb.assertions.Assertions.notNull; import static java.lang.String.format; From ce754d60d427ebc979c2a771b6cd33b65322c200 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 23 Apr 2025 18:53:02 -0700 Subject: [PATCH 18/18] Make computeCStringLength private. --- bson/src/main/org/bson/io/ByteBufferBsonInput.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bson/src/main/org/bson/io/ByteBufferBsonInput.java b/bson/src/main/org/bson/io/ByteBufferBsonInput.java index 8a2e463b6ed..f8be97ddaa7 100644 --- a/bson/src/main/org/bson/io/ByteBufferBsonInput.java +++ b/bson/src/main/org/bson/io/ByteBufferBsonInput.java @@ -182,7 +182,7 @@ public void skipCString() { buffer.position(pos + length); } - public int computeCStringLength(final int prevPos) { + private int computeCStringLength(final int prevPos) { ensureOpen(); int pos = buffer.position(); int limit = buffer.limit();