From a584c9c6d3ca9e9f9b9a0ac5f8768ca5ccffc135 Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Sun, 9 Feb 2025 19:49:58 -0800 Subject: [PATCH 1/8] stash --- bson/build.gradle | 17 ++ .../benchmarks/SerializationBenchmark.java | 170 ++++++++++++++++++ .../src/jmh/java/benchmarks/package-info.java | 20 +++ bson/src/main/org/bson/BsonBinaryWriter.java | 2 +- .../main/org/bson/io/BasicOutputBuffer.java | 75 ++++++-- .../io/BasicOutputBufferSpecification.groovy | 21 ++- 6 files changed, 288 insertions(+), 17 deletions(-) create mode 100644 bson/src/jmh/java/benchmarks/SerializationBenchmark.java create mode 100644 bson/src/jmh/java/benchmarks/package-info.java diff --git a/bson/build.gradle b/bson/build.gradle index d2b2ed3ba0e..41e82976e4c 100644 --- a/bson/build.gradle +++ b/bson/build.gradle @@ -13,10 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +plugins { + id "me.champeau.jmh" version "0.6.3" +} archivesBaseName = 'bson' description = 'The BSON library' +jmh { + includeTests = false + profilers = ["gc"] + failOnError = true +} + +dependencies { + jmh 'org.openjdk.jmh:jmh-core:+' + jmh 'org.openjdk.jmh:jmh-generator-annprocess:+' + + jmhAnnotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess:+' +} + ext { pomName = 'BSON' pomURL = 'https://bsonspec.org' @@ -26,3 +42,4 @@ afterEvaluate { jar.manifest.attributes['Automatic-Module-Name'] = 'org.mongodb.bson' jar.manifest.attributes['Import-Package'] = 'org.slf4j.*;resolution:=optional' } + diff --git a/bson/src/jmh/java/benchmarks/SerializationBenchmark.java b/bson/src/jmh/java/benchmarks/SerializationBenchmark.java new file mode 100644 index 00000000000..4546d06bb89 --- /dev/null +++ b/bson/src/jmh/java/benchmarks/SerializationBenchmark.java @@ -0,0 +1,170 @@ +/* + * 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 benchmarks; + +import java.util.Date; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import org.bson.BsonBinaryWriter; +import org.bson.io.BasicOutputBuffer; +import org.bson.io.OutputBuffer; +import org.bson.types.ObjectId; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Mine + * ---------------------------------------------------------------------------------- + Benchmark Mode Cnt Score Error Units + SerializationBenchmark.toByteArray_large thrpt 10 1.920 ± 0.709 ops/ms + SerializationBenchmark.toByteArray_large:gc.alloc.rate.norm thrpt 10 13_388_928.163 ± 0.107 B/op + SerializationBenchmark.toByteArray_small thrpt 10 209_157.248 ± 8860.801 ops/ms + SerializationBenchmark.toByteArray_small:gc.alloc.rate.norm thrpt 10 80.000 ± 0.001 B/op + SerializationBenchmark.write_id thrpt 10 33_845.743 ± 3292.592 ops/ms + SerializationBenchmark.write_id:gc.alloc.rate.norm thrpt 10 80.000 ± 0.001 B/op + SerializationBenchmark.write_largeDocument thrpt 10 0.024 ± 0.001 ops/ms + SerializationBenchmark.write_largeDocument:gc.alloc.rate.norm thrpt 10 33_120_450.944 ± 5621183.081 B/op + SerializationBenchmark.write_numbers thrpt 10 16_249.730 ± 801.250 ops/ms + SerializationBenchmark.write_numbers:gc.alloc.rate.norm thrpt 10 80.000 ± 0.001 B/op + + * + * Master ---------------------------------------------------------------------------- + * + * Benchmark Mode Cnt Score Error Units + * SerializationBenchmark.toByteArray_large thrpt 10 0.887 ± 0.073 ops/ms + * SerializationBenchmark.toByteArray_large:gc.alloc.rate.norm thrpt 10 26_777_870.800 ± 15.285 B/op + * SerializationBenchmark.toByteArray_small thrpt 10 108_887.377 ± 3914.072 ops/ms + * SerializationBenchmark.toByteArray_small:gc.alloc.rate.norm thrpt 10 160.000 ± 0.001 B/op + * SerializationBenchmark.write_id thrpt 10 24_058.792 ± 1726.869 ops/ms + * SerializationBenchmark.write_id:gc.alloc.rate.norm thrpt 10 112.000 ± 0.001 B/op + * SerializationBenchmark.write_largeDocument thrpt 10 0.032 ± 0.013 ops/ms + * SerializationBenchmark.write_largeDocument:gc.alloc.rate.norm thrpt 10 38_400_448.921 ± 3.630 B/op + * SerializationBenchmark.write_numbers thrpt 10 13_154.730 ± 167.654 ops/ms + * SerializationBenchmark.write_numbers:gc.alloc.rate.norm thrpt 10 80.000 ± 0.001 B/op + * ----------------------------------- + */ +@SuppressWarnings("all") +@BenchmarkMode(Mode.Throughput) +@Warmup(iterations = 2, time = 2, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 2, time = 2, timeUnit = TimeUnit.SECONDS) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +public class SerializationBenchmark { + + @State(Scope.Thread) + public static class Input { + + public OutputBuffer buffer, smallBuffer, largeBuffer; + public BsonBinaryWriter writer; + public int num; + public double d; + public ObjectId id; + + @Setup + public void setup() { + this.buffer = new BasicOutputBuffer(); + this.largeBuffer = new BasicOutputBuffer(); + this.smallBuffer = new BasicOutputBuffer(); + + this.num = ThreadLocalRandom.current().nextInt(); + this.d = ThreadLocalRandom.current().nextDouble(); + this.id = new ObjectId(); + + this.writer = new BsonBinaryWriter(this.buffer); + this.writer.writeStartDocument(); + this.writer.mark(); + + try (BsonBinaryWriter writer = new BsonBinaryWriter(this.largeBuffer)) { + writer.writeStartDocument(); + writer.writeStartArray("array"); + for (int i = 0; i < 300_000; ++i) { + writeDoc(writer); + } + writer.writeEndArray(); + writer.writeEndDocument(); + } + + try (BsonBinaryWriter writer = new BsonBinaryWriter(this.smallBuffer)) { + writer.writeStartDocument(); + writer.writeStartArray("array"); + writeDoc(writer); + writer.writeEndArray(); + writer.writeEndDocument(); + } + } + + private void writeDoc(BsonBinaryWriter writer) { + writer.writeStartDocument(); + writer.writeObjectId("_id", this.id); + writer.writeDouble("value", ThreadLocalRandom.current().nextDouble()); + writer.writeEndDocument(); + } + } + + @Benchmark + public void write_numbers(Input input, Blackhole blackhole) { + BsonBinaryWriter writer = input.writer; + writer.writeInt32("a", input.num); + writer.writeInt64("b", input.num); + writer.writeDouble("c", input.d); + writer.writeDateTime("d", input.num); + writer.reset(); + writer.mark(); + blackhole.consume(input.buffer); + } + + @Benchmark + public void write_id(Input input, Blackhole blackhole) { + BsonBinaryWriter writer = input.writer; + writer.writeObjectId("a", input.id); + writer.reset(); + writer.mark(); + blackhole.consume(input.buffer); + } + + @Benchmark + public void toByteArray_large(Input input, Blackhole blackhole) { + blackhole.consume(input.largeBuffer.toByteArray()); + } + + @Benchmark + public void toByteArray_small(Input input, Blackhole blackhole) { + blackhole.consume(input.smallBuffer.toByteArray()); + } + + @Benchmark + public void write_largeDocument(Input input, Blackhole blackhole) { + try (BsonBinaryWriter writer = new BsonBinaryWriter(input.largeBuffer)) { + writer.writeStartDocument(); + writer.writeStartArray("a"); + for (int i = 0; i < 300_000; ++i) { + input.writeDoc(writer); + } + writer.writeEndArray(); + writer.writeEndDocument(); + } + blackhole.consume(input.largeBuffer); + input.largeBuffer.truncateToPosition(0); + } +} diff --git a/bson/src/jmh/java/benchmarks/package-info.java b/bson/src/jmh/java/benchmarks/package-info.java new file mode 100644 index 00000000000..c356e2494c9 --- /dev/null +++ b/bson/src/jmh/java/benchmarks/package-info.java @@ -0,0 +1,20 @@ +/* + * 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. + */ + +/** + * Contains classes implementing I/O operations used by BSON objects. + */ +package benchmarks; diff --git a/bson/src/main/org/bson/BsonBinaryWriter.java b/bson/src/main/org/bson/BsonBinaryWriter.java index d9301fd5cb3..ab79d9803f4 100644 --- a/bson/src/main/org/bson/BsonBinaryWriter.java +++ b/bson/src/main/org/bson/BsonBinaryWriter.java @@ -263,7 +263,7 @@ public void doWriteNull() { public void doWriteObjectId(final ObjectId value) { bsonOutput.writeByte(BsonType.OBJECT_ID.getValue()); writeCurrentName(); - bsonOutput.writeBytes(value.toByteArray()); + bsonOutput.writeObjectId(value); } @Override diff --git a/bson/src/main/org/bson/io/BasicOutputBuffer.java b/bson/src/main/org/bson/io/BasicOutputBuffer.java index 8227940182e..9a92f46f93e 100644 --- a/bson/src/main/org/bson/io/BasicOutputBuffer.java +++ b/bson/src/main/org/bson/io/BasicOutputBuffer.java @@ -18,11 +18,14 @@ import org.bson.ByteBuf; import org.bson.ByteBufNIO; +import org.bson.types.ObjectId; import java.io.IOException; import java.io.OutputStream; +import java.nio.Buffer; import java.nio.ByteBuffer; import java.util.Arrays; +import java.util.Collections; import java.util.List; import static java.lang.String.format; @@ -35,10 +38,14 @@ public class BasicOutputBuffer extends OutputBuffer { private byte[] buffer; private int position; - /** - * Construct an instance with a default initial byte array size. - */ - public BasicOutputBuffer() { + /** + * A wrapper around `buffer` used to write ObjectIDs without allocating temprary arrays and + * leverage JVM intrinsics for writing little-endian numeric values. + */ + private ByteBuffer buf; + + /** Construct an instance with a default initial byte array size. */ + public BasicOutputBuffer() { this(1024); } @@ -49,6 +56,7 @@ public BasicOutputBuffer() { */ public BasicOutputBuffer(final int initialSize) { buffer = new byte[initialSize]; + buf = ByteBuffer.wrap(buffer).order(LITTLE_ENDIAN); } /** @@ -67,6 +75,44 @@ public void write(final byte[] b) { write(b, 0, b.length); } + @Override + public byte[] toByteArray() { + ensureOpen(); + return Arrays.copyOf(buffer, position); + } + + @Override + public void writeInt32(final int value) { + ensureOpen(); + ensure(4); + buf.putInt(position, value); + position += 4; + } + + @Override + public void writeInt32(final int position, final int value) { + ensureOpen(); + checkPosition(position, 4); + buf.putInt(position, value); + } + + @Override + public void writeInt64(final long value) { + ensureOpen(); + ensure(8); + buf.putLong(position, value); + position += 8; + } + + @Override + public void writeObjectId(final ObjectId value) { + ensureOpen(); + ensure(12); + ((Buffer) buf).position(position); // Avoid covariant call on jdk8 + value.putToByteBuffer(buf); + position += 12; + } + @Override public void writeBytes(final byte[] bytes, final int offset, final int length) { ensureOpen(); @@ -87,13 +133,7 @@ public void writeByte(final int value) { @Override protected void write(final int absolutePosition, final int value) { ensureOpen(); - - if (absolutePosition < 0) { - throw new IllegalArgumentException(format("position must be >= 0 but was %d", absolutePosition)); - } - if (absolutePosition > position - 1) { - throw new IllegalArgumentException(format("position must be <= %d but was %d", position - 1, absolutePosition)); - } + checkPosition(absolutePosition, 1); buffer[absolutePosition] = (byte) (0xFF & value); } @@ -132,7 +172,7 @@ public void truncateToPosition(final int newPosition) { @Override public List getByteBuffers() { ensureOpen(); - return Arrays.asList(new ByteBufNIO(ByteBuffer.wrap(buffer, 0, position).duplicate().order(LITTLE_ENDIAN))); + return Collections.singletonList(new ByteBufNIO(ByteBuffer.wrap(buffer, 0, position).duplicate().order(LITTLE_ENDIAN))); } @Override @@ -160,6 +200,17 @@ private void ensure(final int more) { byte[] n = new byte[newSize]; System.arraycopy(buffer, 0, n, 0, position); buffer = n; + buf = ByteBuffer.wrap(buffer).order(LITTLE_ENDIAN); + } + + /** Ensures that `absolutePosition` is a valid index in `this.buffer` and there is room to write at least `bytesToWrite` bytes. */ + private void checkPosition(final int absolutePosition, final int bytesToWrite) { + if (absolutePosition < 0) { + throw new IllegalArgumentException(format("position must be >= 0 but was %d", absolutePosition)); + } + if (absolutePosition > position - bytesToWrite) { + throw new IllegalArgumentException(format("position must be <= %d but was %d", position - 1, absolutePosition)); + } } } diff --git a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy index 38de06bf8cf..e0d568c943d 100644 --- a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy +++ b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy @@ -72,7 +72,7 @@ class BasicOutputBufferSpecification extends Specification { def 'should write a little endian Int32'() { given: - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(3) when: bsonOutput.writeInt32(0x1020304) @@ -85,7 +85,7 @@ class BasicOutputBufferSpecification extends Specification { def 'should write a little endian Int64'() { given: - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(7) when: bsonOutput.writeInt64(0x102030405060708L) @@ -98,7 +98,7 @@ class BasicOutputBufferSpecification extends Specification { def 'should write a double'() { given: - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(7) when: bsonOutput.writeDouble(Double.longBitsToDouble(0x102030405060708L)) @@ -112,7 +112,7 @@ class BasicOutputBufferSpecification extends Specification { def 'should write an ObjectId'() { given: def objectIdAsByteArray = [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1] as byte[] - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(11) when: bsonOutput.writeObjectId(new ObjectId(objectIdAsByteArray)) @@ -123,6 +123,19 @@ class BasicOutputBufferSpecification extends Specification { bsonOutput.size == 12 } + def 'write ObjectId should throw after close'() { + given: + def objectIdAsByteArray = [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1] as byte[] + def bsonOutput = new BasicOutputBuffer() + bsonOutput.close() + + when: + bsonOutput.writeObjectId(new ObjectId(objectIdAsByteArray)) + + then: + thrown(IllegalStateException) + } + def 'should write an empty string'() { given: def bsonOutput = new BasicOutputBuffer() From 2ad83857445630f77d4fee61494d1fe0642ab793 Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Sun, 9 Feb 2025 21:53:52 -0800 Subject: [PATCH 2/8] --amend --- .../main/org/bson/io/BasicOutputBuffer.java | 98 ++++++++++--------- 1 file changed, 51 insertions(+), 47 deletions(-) diff --git a/bson/src/main/org/bson/io/BasicOutputBuffer.java b/bson/src/main/org/bson/io/BasicOutputBuffer.java index 9a92f46f93e..9d164c2767f 100644 --- a/bson/src/main/org/bson/io/BasicOutputBuffer.java +++ b/bson/src/main/org/bson/io/BasicOutputBuffer.java @@ -38,14 +38,16 @@ public class BasicOutputBuffer extends OutputBuffer { private byte[] buffer; private int position; - /** - * A wrapper around `buffer` used to write ObjectIDs without allocating temprary arrays and - * leverage JVM intrinsics for writing little-endian numeric values. - */ - private ByteBuffer buf; - - /** Construct an instance with a default initial byte array size. */ - public BasicOutputBuffer() { + /** + * A wrapper around `buffer` used to write ObjectIDs without allocating temporary arrays and + * leverage JVM intrinsics for writing little-endian numeric values. + */ + private ByteBuffer buf; + + /** + * Construct an instance with a default initial byte array size. + */ + public BasicOutputBuffer() { this(1024); } @@ -75,43 +77,43 @@ public void write(final byte[] b) { write(b, 0, b.length); } - @Override - public byte[] toByteArray() { - ensureOpen(); - return Arrays.copyOf(buffer, position); - } - - @Override - public void writeInt32(final int value) { - ensureOpen(); - ensure(4); - buf.putInt(position, value); - position += 4; - } - - @Override - public void writeInt32(final int position, final int value) { - ensureOpen(); - checkPosition(position, 4); - buf.putInt(position, value); - } - - @Override - public void writeInt64(final long value) { - ensureOpen(); - ensure(8); - buf.putLong(position, value); - position += 8; - } - - @Override - public void writeObjectId(final ObjectId value) { - ensureOpen(); - ensure(12); - ((Buffer) buf).position(position); // Avoid covariant call on jdk8 - value.putToByteBuffer(buf); - position += 12; - } + @Override + public byte[] toByteArray() { + ensureOpen(); + return Arrays.copyOf(buffer, position); + } + + @Override + public void writeInt32(final int value) { + ensureOpen(); + ensure(4); + buf.putInt(position, value); + position += 4; + } + + @Override + public void writeInt32(final int position, final int value) { + ensureOpen(); + checkPosition(position, 4); + buf.putInt(position, value); + } + + @Override + public void writeInt64(final long value) { + ensureOpen(); + ensure(8); + buf.putLong(position, value); + position += 8; + } + + @Override + public void writeObjectId(final ObjectId value) { + ensureOpen(); + ensure(12); + ((Buffer) buf).position(position); // Avoid covariant call on jdk8 + value.putToByteBuffer(buf); + position += 12; + } @Override public void writeBytes(final byte[] bytes, final int offset, final int length) { @@ -203,7 +205,10 @@ private void ensure(final int more) { buf = ByteBuffer.wrap(buffer).order(LITTLE_ENDIAN); } - /** Ensures that `absolutePosition` is a valid index in `this.buffer` and there is room to write at least `bytesToWrite` bytes. */ + /** + * Ensures that `absolutePosition` is a valid index in `this.buffer` and there is room to write at + * least `bytesToWrite` bytes. + */ private void checkPosition(final int absolutePosition, final int bytesToWrite) { if (absolutePosition < 0) { throw new IllegalArgumentException(format("position must be >= 0 but was %d", absolutePosition)); @@ -212,5 +217,4 @@ private void checkPosition(final int absolutePosition, final int bytesToWrite) { throw new IllegalArgumentException(format("position must be <= %d but was %d", position - 1, absolutePosition)); } } - } From 0385da6e479a135bc844d9292d15a9f9ac4f093d Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Mon, 10 Feb 2025 14:08:52 -0800 Subject: [PATCH 3/8] use bytebuffer for all implementations + extra tests --- .../main/org/bson/io/BasicOutputBuffer.java | 72 +++++++++---------- .../io/BasicOutputBufferSpecification.groovy | 51 ++++++++++++- 2 files changed, 81 insertions(+), 42 deletions(-) diff --git a/bson/src/main/org/bson/io/BasicOutputBuffer.java b/bson/src/main/org/bson/io/BasicOutputBuffer.java index 9d164c2767f..0672408168b 100644 --- a/bson/src/main/org/bson/io/BasicOutputBuffer.java +++ b/bson/src/main/org/bson/io/BasicOutputBuffer.java @@ -35,14 +35,12 @@ * A BSON output stream that stores the output in a single, un-pooled byte array. */ public class BasicOutputBuffer extends OutputBuffer { - private byte[] buffer; - private int position; /** - * A wrapper around `buffer` used to write ObjectIDs without allocating temporary arrays and - * leverage JVM intrinsics for writing little-endian numeric values. + * This ByteBuffer allows us to write ObjectIDs without allocating a temporary array per object, and enables us + * to leverage JVM intrinsics for writing little-endian numeric values. */ - private ByteBuffer buf; + private ByteBuffer buffer; /** * Construct an instance with a default initial byte array size. @@ -57,8 +55,8 @@ public BasicOutputBuffer() { * @param initialSize the initial size of the byte array */ public BasicOutputBuffer(final int initialSize) { - buffer = new byte[initialSize]; - buf = ByteBuffer.wrap(buffer).order(LITTLE_ENDIAN); + // Allocate heap buffer to ensure we can access underlying array + buffer = ByteBuffer.allocate(initialSize).order(LITTLE_ENDIAN); } /** @@ -68,51 +66,46 @@ public BasicOutputBuffer(final int initialSize) { * @since 3.3 */ public byte[] getInternalBuffer() { - return buffer; + return buffer.array(); } @Override public void write(final byte[] b) { - ensureOpen(); - write(b, 0, b.length); + writeBytes(b, 0, b.length); } @Override public byte[] toByteArray() { ensureOpen(); - return Arrays.copyOf(buffer, position); + return Arrays.copyOf(buffer.array(), buffer.position()); } @Override public void writeInt32(final int value) { ensureOpen(); ensure(4); - buf.putInt(position, value); - position += 4; + buffer.putInt(value); } @Override public void writeInt32(final int position, final int value) { ensureOpen(); checkPosition(position, 4); - buf.putInt(position, value); + buffer.putInt(position, value); } @Override public void writeInt64(final long value) { ensureOpen(); ensure(8); - buf.putLong(position, value); - position += 8; + buffer.putLong(value); } @Override public void writeObjectId(final ObjectId value) { ensureOpen(); ensure(12); - ((Buffer) buf).position(position); // Avoid covariant call on jdk8 - value.putToByteBuffer(buf); - position += 12; + value.putToByteBuffer(buffer); } @Override @@ -120,8 +113,7 @@ public void writeBytes(final byte[] bytes, final int offset, final int length) { ensureOpen(); ensure(length); - System.arraycopy(bytes, offset, buffer, position, length); - position += length; + buffer.put(bytes, offset, length); } @Override @@ -129,7 +121,7 @@ public void writeByte(final int value) { ensureOpen(); ensure(1); - buffer[position++] = (byte) (0xFF & value); + buffer.put((byte) (0xFF & value)); } @Override @@ -137,13 +129,13 @@ protected void write(final int absolutePosition, final int value) { ensureOpen(); checkPosition(absolutePosition, 1); - buffer[absolutePosition] = (byte) (0xFF & value); + buffer.put(absolutePosition, (byte) (0xFF & value)); } @Override public int getPosition() { ensureOpen(); - return position; + return buffer.position(); } /** @@ -152,29 +144,31 @@ public int getPosition() { @Override public int getSize() { ensureOpen(); - return position; + return buffer.position(); } @Override public int pipe(final OutputStream out) throws IOException { ensureOpen(); - out.write(buffer, 0, position); - return position; + out.write(buffer.array(), 0, buffer.position()); + return buffer.position(); } @Override public void truncateToPosition(final int newPosition) { ensureOpen(); - if (newPosition > position || newPosition < 0) { + if (newPosition > buffer.position() || newPosition < 0) { throw new IllegalArgumentException(); } - position = newPosition; + ((Buffer) buffer).position(newPosition); } @Override public List getByteBuffers() { ensureOpen(); - return Collections.singletonList(new ByteBufNIO(ByteBuffer.wrap(buffer, 0, position).duplicate().order(LITTLE_ENDIAN))); + // Create a flipped copy of the buffer for reading. Note that ByteBufNIO overwrites the endian-ness. + ByteBuffer flipped = ByteBuffer.wrap(buffer.array(), 0, buffer.position()); + return Collections.singletonList(new ByteBufNIO(flipped)); } @Override @@ -189,20 +183,20 @@ private void ensureOpen() { } private void ensure(final int more) { - int need = position + more; - if (need <= buffer.length) { + int length = buffer.position(); + int need = length + more; + if (need <= buffer.capacity()) { return; } - int newSize = buffer.length * 2; + int newSize = length * 2; if (newSize < need) { newSize = need + 128; } - byte[] n = new byte[newSize]; - System.arraycopy(buffer, 0, n, 0, position); - buffer = n; - buf = ByteBuffer.wrap(buffer).order(LITTLE_ENDIAN); + ByteBuffer tmp = ByteBuffer.allocate(newSize).order(LITTLE_ENDIAN); + tmp.put(buffer.array(), 0, length); // Avoids covariant call to flip on jdk8 + this.buffer = tmp; } /** @@ -213,8 +207,8 @@ private void checkPosition(final int absolutePosition, final int bytesToWrite) { if (absolutePosition < 0) { throw new IllegalArgumentException(format("position must be >= 0 but was %d", absolutePosition)); } - if (absolutePosition > position - bytesToWrite) { - throw new IllegalArgumentException(format("position must be <= %d but was %d", position - 1, absolutePosition)); + if (absolutePosition > buffer.position() - bytesToWrite) { + throw new IllegalArgumentException(format("position must be <= %d but was %d", buffer.position() - 1, absolutePosition)); } } } diff --git a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy index e0d568c943d..7efeb80b2e5 100644 --- a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy +++ b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy @@ -17,6 +17,7 @@ package org.bson.io import org.bson.BsonSerializationException +import org.bson.ByteBuf import org.bson.types.ObjectId import spock.lang.Specification @@ -44,9 +45,22 @@ class BasicOutputBufferSpecification extends Specification { bsonOutput.size == 1 } + def 'writeBytes shorthand should extend buffer'() { + given: + def bsonOutput = new BasicOutputBuffer(3) + + when: + bsonOutput.write([1, 2, 3, 4] as byte[]) + + then: + getBytes(bsonOutput) == [1, 2, 3, 4] as byte[] + bsonOutput.position == 4 + bsonOutput.size == 4 + } + def 'should write bytes'() { given: - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(3) when: bsonOutput.writeBytes([1, 2, 3, 4] as byte[]) @@ -59,7 +73,7 @@ class BasicOutputBufferSpecification extends Specification { def 'should write bytes from offset until length'() { given: - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(5) when: bsonOutput.writeBytes([0, 1, 2, 3, 4, 5] as byte[], 1, 4) @@ -70,6 +84,23 @@ class BasicOutputBufferSpecification extends Specification { bsonOutput.size == 4 } + def 'toByteArray should be idempotent'() { + given: + def bsonOutput = new BasicOutputBuffer(10) + bsonOutput.writeBytes([1, 2, 3, 4] as byte[]) + + when: + def first = bsonOutput.toByteArray() + def second = bsonOutput.toByteArray() + + then: + getBytes(bsonOutput) == [1, 2, 3, 4] as byte[] + first == [1, 2, 3, 4] as byte[] + second == [1, 2, 3, 4] as byte[] + bsonOutput.position == 4 + bsonOutput.size == 4 + } + def 'should write a little endian Int32'() { given: def bsonOutput = new BasicOutputBuffer(3) @@ -164,7 +195,7 @@ class BasicOutputBufferSpecification extends Specification { def 'should write a UTF-8 string'() { given: - def bsonOutput = new BasicOutputBuffer() + def bsonOutput = new BasicOutputBuffer(7) when: bsonOutput.writeString('\u0900') @@ -333,6 +364,20 @@ class BasicOutputBufferSpecification extends Specification { bsonOutput.getByteBuffers()[0].getInt() == 1 } + def 'should get byte buffer with limit'() { + given: + def bsonOutput = new BasicOutputBuffer(8) + bsonOutput.writeBytes([1, 0, 0, 0] as byte[]) + + when: + def buffers = bsonOutput.getByteBuffers() + + then: + buffers.size() == 1 + buffers[0].position() == 0 + buffers[0].limit() == 4 + } + def 'should get internal buffer'() { given: def bsonOutput = new BasicOutputBuffer(4) From aa1c2f6b13605a3021870e490cb17e0d189b90f5 Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Mon, 10 Feb 2025 19:26:23 -0800 Subject: [PATCH 4/8] fix imports --- .../test/unit/org/bson/io/BasicOutputBufferSpecification.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy index 7efeb80b2e5..7ea73ddda2a 100644 --- a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy +++ b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy @@ -17,7 +17,6 @@ package org.bson.io import org.bson.BsonSerializationException -import org.bson.ByteBuf import org.bson.types.ObjectId import spock.lang.Specification From 1eaffc2cc1308d26f3b24b304217f57bc04ec173 Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Sat, 5 Apr 2025 19:35:16 -0700 Subject: [PATCH 5/8] Update bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy Co-authored-by: Viacheslav Babanin --- .../bson/io/BasicOutputBufferSpecification.groovy | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy index 7ea73ddda2a..74135bcfeba 100644 --- a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy +++ b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy @@ -100,6 +100,20 @@ class BasicOutputBufferSpecification extends Specification { bsonOutput.size == 4 } + def 'toByteArray creates a copy'() { + given: + def bsonOutput = new BasicOutputBuffer(10) + bsonOutput.writeBytes([1, 2, 3, 4] as byte[]) + + when: + def first = bsonOutput.toByteArray() + def second = bsonOutput.toByteArray() + + then: + first !== second + first == [1, 2, 3, 4] as byte[] + second == [1, 2, 3, 4] as byte[] + } def 'should write a little endian Int32'() { given: def bsonOutput = new BasicOutputBuffer(3) From 0ca525dfbb43d3de08d8e5b31a7c6b7aa1ce162d Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Sat, 5 Apr 2025 19:35:32 -0700 Subject: [PATCH 6/8] Update bson/src/main/org/bson/io/BasicOutputBuffer.java Co-authored-by: Viacheslav Babanin --- bson/src/main/org/bson/io/BasicOutputBuffer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/bson/src/main/org/bson/io/BasicOutputBuffer.java b/bson/src/main/org/bson/io/BasicOutputBuffer.java index 0672408168b..8c3292b4bcf 100644 --- a/bson/src/main/org/bson/io/BasicOutputBuffer.java +++ b/bson/src/main/org/bson/io/BasicOutputBuffer.java @@ -160,6 +160,7 @@ public void truncateToPosition(final int newPosition) { if (newPosition > buffer.position() || newPosition < 0) { throw new IllegalArgumentException(); } + // The cast is required for compatibility with JDK 9+ where ByteBuffer's position method is inherited from Buffer. ((Buffer) buffer).position(newPosition); } From 9c3aae3199b3bacfaca18e0c4bab9720702cfba5 Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Sat, 5 Apr 2025 19:35:39 -0700 Subject: [PATCH 7/8] Update bson/src/main/org/bson/io/BasicOutputBuffer.java Co-authored-by: Viacheslav Babanin --- bson/src/main/org/bson/io/BasicOutputBuffer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bson/src/main/org/bson/io/BasicOutputBuffer.java b/bson/src/main/org/bson/io/BasicOutputBuffer.java index 8c3292b4bcf..aaff34d6476 100644 --- a/bson/src/main/org/bson/io/BasicOutputBuffer.java +++ b/bson/src/main/org/bson/io/BasicOutputBuffer.java @@ -209,7 +209,7 @@ private void checkPosition(final int absolutePosition, final int bytesToWrite) { throw new IllegalArgumentException(format("position must be >= 0 but was %d", absolutePosition)); } if (absolutePosition > buffer.position() - bytesToWrite) { - throw new IllegalArgumentException(format("position must be <= %d but was %d", buffer.position() - 1, absolutePosition)); + throw new IllegalArgumentException(format("position must be <= %d but was %d", buffer.position() - bytesToWrite, absolutePosition)); } } } From 9a187bd533f57afa1b8ad3ab7b5c963d8911e9df Mon Sep 17 00:00:00 2001 From: Evan Darke Date: Sat, 5 Apr 2025 20:06:05 -0700 Subject: [PATCH 8/8] Add test for absolute write --- .../io/BasicOutputBufferSpecification.groovy | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy index 74135bcfeba..758d4fc1cfd 100644 --- a/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy +++ b/bson/src/test/unit/org/bson/io/BasicOutputBufferSpecification.groovy @@ -320,6 +320,46 @@ class BasicOutputBufferSpecification extends Specification { bsonOutput.size == 8 } + def 'absolute write should throw with invalid position'() { + given: + def bsonOutput = new BasicOutputBuffer() + bsonOutput.writeBytes([1, 2, 3, 4] as byte[]) + + when: + bsonOutput.write(-1, 0x1020304) + + then: + thrown(IllegalArgumentException) + + when: + bsonOutput.write(4, 0x1020304) + + then: + thrown(IllegalArgumentException) + } + + def 'absolute write should write lower byte at position'() { + given: + def bsonOutput = new BasicOutputBuffer() + bsonOutput.writeBytes([0, 0, 0, 0, 1, 2, 3, 4] as byte[]) + + when: + bsonOutput.write(0, 0x1020304) + + then: + getBytes(bsonOutput) == [4, 0, 0, 0, 1, 2, 3, 4] as byte[] + bsonOutput.position == 8 + bsonOutput.size == 8 + + when: + bsonOutput.write(7, 0x1020304) + + then: + getBytes(bsonOutput) == [4, 0, 0, 0, 1, 2, 3, 4] as byte[] + bsonOutput.position == 8 + bsonOutput.size == 8 + } + def 'truncate should throw with invalid position'() { given: def bsonOutput = new BasicOutputBuffer()