-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Optimize RawBsonDocument encode and decode #1913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
cd52b6b
03978bf
fc3726d
a543455
81ec442
a5790af
103f461
9f49d2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -334,6 +334,26 @@ public void pipe(final BsonReader reader) { | |
| pipeDocument(reader, null); | ||
| } | ||
|
|
||
| /** | ||
| * Pipes a raw BSON document from the given byte array to this writer, writing the bytes directly to the | ||
| * output without intermediate object allocation. | ||
| * | ||
| * @param bytes the byte array containing the BSON document | ||
| * @param offset the offset into the byte array | ||
| * @param length the length of the BSON document | ||
| * @since 5.8 | ||
| */ | ||
| public void pipe(final byte[] bytes, final int offset, final int length) { | ||
| checkMinDocumentSize(length); | ||
| if (getState() == State.VALUE) { | ||
| bsonOutput.writeByte(BsonType.DOCUMENT.getValue()); | ||
| writeCurrentName(); | ||
| } | ||
| int pipedDocumentStartPosition = bsonOutput.getPosition(); | ||
| bsonOutput.writeBytes(bytes, offset, length); | ||
| completePipeDocument(pipedDocumentStartPosition); | ||
|
vbabanin marked this conversation as resolved.
|
||
| } | ||
|
|
||
| @Override | ||
| public void pipe(final BsonReader reader, final List<BsonElement> extraElements) { | ||
| notNull("reader", reader); | ||
|
|
@@ -350,14 +370,10 @@ private void pipeDocument(final BsonReader reader, final List<BsonElement> extra | |
| } | ||
| BsonInput bsonInput = binaryReader.getBsonInput(); | ||
| int size = bsonInput.readInt32(); | ||
| if (size < 5) { | ||
| throw new BsonSerializationException("Document size must be at least 5"); | ||
| } | ||
| checkMinDocumentSize(size); | ||
| int pipedDocumentStartPosition = bsonOutput.getPosition(); | ||
| bsonOutput.writeInt32(size); | ||
| byte[] bytes = new byte[size - 4]; | ||
| bsonInput.readBytes(bytes); | ||
| bsonOutput.writeBytes(bytes); | ||
|
Comment on lines
-358
to
-360
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This avoids an extra temporary byte[] allocation by reading directly into the target buffer, reducing both allocation pressure and byte-copy/processing overhead. |
||
| bsonInput.pipe(bsonOutput, size - 4); | ||
|
|
||
| binaryReader.setState(AbstractBsonReader.State.TYPE); | ||
|
|
||
|
|
@@ -371,24 +387,27 @@ private void pipeDocument(final BsonReader reader, final List<BsonElement> extra | |
| setContext(getContext().getParentContext()); | ||
| } | ||
|
|
||
| if (getContext() == null) { | ||
| setState(State.DONE); | ||
| } else { | ||
| if (getContext().getContextType() == BsonContextType.JAVASCRIPT_WITH_SCOPE) { | ||
| backpatchSize(); // size of the JavaScript with scope value | ||
| setContext(getContext().getParentContext()); | ||
| } | ||
| setState(getNextState()); | ||
| } | ||
|
|
||
| validateSize(bsonOutput.getPosition() - pipedDocumentStartPosition); | ||
| completePipeDocument(pipedDocumentStartPosition); | ||
| } else if (extraElements != null) { | ||
| super.pipe(reader, extraElements); | ||
| } else { | ||
| super.pipe(reader); | ||
| } | ||
| } | ||
|
|
||
| private void completePipeDocument(final int pipedDocumentStartPosition) { | ||
| if (getContext() == null) { | ||
| setState(State.DONE); | ||
| } else { | ||
| if (getContext().getContextType() == BsonContextType.JAVASCRIPT_WITH_SCOPE) { | ||
| backpatchSize(); // size of the JavaScript with scope value | ||
| setContext(getContext().getParentContext()); | ||
| } | ||
| setState(getNextState()); | ||
| } | ||
| validateSize(bsonOutput.getPosition() - pipedDocumentStartPosition); | ||
| } | ||
|
|
||
| /** | ||
| * Sets a maximum size for documents from this point. | ||
| * | ||
|
|
@@ -426,6 +445,12 @@ public void reset() { | |
| mark = null; | ||
| } | ||
|
|
||
| private static void checkMinDocumentSize(final int size) { | ||
| if (size < 5) { | ||
| throw new BsonSerializationException("Document size must be at least 5"); | ||
| } | ||
| } | ||
|
|
||
| private void writeCurrentName() { | ||
| if (getContext().getContextType() == BsonContextType.ARRAY) { | ||
| int index = getContext().index++; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -356,5 +356,4 @@ public interface BsonWriter { | |
| * @param reader The source. | ||
| */ | ||
| void pipe(BsonReader reader); | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -802,7 +802,34 @@ public void testPipeOfDocumentWithInvalidSize() { | |
| // expected | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testPipeOfRawBytes() { | ||
| BasicOutputBuffer sourceBuffer = new BasicOutputBuffer(); | ||
| try (BsonBinaryWriter sourceWriter = new BsonBinaryWriter(sourceBuffer)) { | ||
| sourceWriter.writeStartDocument(); | ||
| sourceWriter.writeBoolean("a", true); | ||
| sourceWriter.writeEndDocument(); | ||
| } | ||
| byte[] documentBytes = sourceBuffer.toByteArray(); | ||
|
|
||
| BasicOutputBuffer destBuffer = new BasicOutputBuffer(); | ||
| try (BsonBinaryWriter destWriter = new BsonBinaryWriter(destBuffer)) { | ||
| destWriter.pipe(documentBytes, 0, documentBytes.length); | ||
| } | ||
|
|
||
| assertArrayEquals(documentBytes, destBuffer.toByteArray()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testPipeOfRawBytesWithInvalidSize() { | ||
| byte[] bytes = {4, 0, 0, 0}; // minimum document size is 5 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw the validation for size of 5, just curious why is it 5 ? |
||
|
|
||
| BasicOutputBuffer newBuffer = new BasicOutputBuffer(); | ||
| try (BsonBinaryWriter newWriter = new BsonBinaryWriter(newBuffer)) { | ||
| assertThrows(BsonSerializationException.class, () -> newWriter.pipe(bytes, 0, bytes.length)); | ||
| } | ||
| } | ||
|
|
||
| // CHECKSTYLE:OFF | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,6 +113,23 @@ class RawBsonDocumentSpecification extends Specification { | |
| rawDocument << createRawDocumentVariants() | ||
| } | ||
|
|
||
|
|
||
| def 'getBackingArray, getByteOffset and getByteLength should expose the document range'() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might be mistaken but I saw a lot of PRs that also remove groovy spec class, can we move this test case to java test instead ? |
||
| expect: | ||
| rawDocument.getByteOffset() == expectedOffset | ||
| rawDocument.getByteLength() == expectedLength | ||
| Arrays.copyOfRange( | ||
| rawDocument.getBackingArray(), | ||
| rawDocument.getByteOffset(), | ||
| rawDocument.getByteOffset() + rawDocument.getByteLength()) == getBytesFromDocument() | ||
|
|
||
| where: | ||
| rawDocument | expectedOffset | expectedLength | ||
| createRawDocumenFromDocument() | 0 | 66 | ||
| createRawDocumentFromByteArray() | 0 | 66 | ||
| createRawDocumentFromByteArrayOffsetLength()| 1 | 66 | ||
| } | ||
|
|
||
| def 'parse should through if parameter is invalid'() { | ||
| when: | ||
| RawBsonDocument.parse(null) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.