perf(NODE-7660): serialize bulk write documents a single time#4999
perf(NODE-7660): serialize bulk write documents a single time#4999tadjik1 wants to merge 11 commits into
Conversation
9938e75 to
343bc2b
Compare
insertMany/bulkWrite serialized each document twice: once via BSON.calculateObjectSize for the max-size check and byte-based batch splitting, then again when the command was serialized. Now each op is serialized once in addToOperationsList (using the bulk op's resolved BSON options), the buffer is stored on the Batch and reused as an OP_MSG payload type-1 document sequence in Insert/Update/Delete buildCommandDocument.
343bc2b to
e4f98ee
Compare
Reassign each batch's serializedOperations to an empty array once the batch has been executed so retained serialized bytes stay bounded to roughly one in-flight batch rather than the entire bulk write payload. The operation keeps its own reference for retries, so this only drops the bulk operation's copy.
There was a problem hiding this comment.
Pull request overview
This PR optimizes legacy bulk write paths (insertMany() / initialize*BulkOp() / bulkWrite()) by serializing each operation document exactly once, reusing the pre-serialized BSON bytes to build OP_MSG payload type 1 document sequences (instead of re-serializing during command construction). It also ensures command monitoring continues to report array-shaped documents/updates/deletes fields even when the wire message uses document sequences.
Changes:
- Add a helper to build
DocumentSequenceobjects from pre-serialized BSON and update insert/update/delete command builders to use it when buffers are available. - Serialize bulk operation documents once during batch building, store buffers on
Batch, and reuse them when constructing write commands (except under auto-encryption). - Update command monitoring extraction + tests to reconstruct document sequences into arrays and validate
ignoreUndefineddefault behavior remains unchanged.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/operations/write_ops.test.ts | Adds unit coverage that update/delete operations use DocumentSequence when buffers are provided. |
| test/unit/operations/insert.test.ts | Adds unit coverage that insert uses DocumentSequence when buffers are provided and arrays otherwise. |
| test/unit/cmap/command_monitoring_events.test.js | Adds unit coverage ensuring command monitoring reconstructs document sequences into arrays. |
| test/unit/assorted/collations.test.js | Adapts a mock-server test to handle updates arriving as a document sequence. |
| test/integration/crud/bulk.test.ts | Adds integration coverage for ignoreUndefined default behavior, monitoring shape, and buffer release semantics. |
| src/operations/update.ts | Allows update command building to use a DocumentSequence when pre-serialized statements are provided. |
| src/operations/insert.ts | Allows insert command building to use a DocumentSequence when pre-serialized documents are provided. |
| src/operations/delete.ts | Allows delete command building to use a DocumentSequence when pre-serialized statements are provided. |
| src/cmap/commands.ts | Adds makeDocumentSequence() to build document sequences without re-serializing. |
| src/cmap/command_monitoring_events.ts | Converts any top-level DocumentSequence fields back into arrays for monitoring events. |
| src/bulk/unordered.ts | Serializes each operation once during batch construction and stores the buffer for reuse. |
| src/bulk/ordered.ts | Serializes each operation once during batch construction and stores the buffer for reuse. |
| src/bulk/common.ts | Stores per-operation serialized buffers on Batch, passes them into operations when allowed, and clears buffers after execution. |
…ncryption Under auto-encryption the command is sent as a BSON array (not a document sequence), so pre-serialized buffers are never reused. Measure the document size with calculateObjectSize instead of a full serialize and do not retain buffers on the batch, restoring the pre-document-sequence behavior for the encrypted path.
nbbeeken
left a comment
There was a problem hiding this comment.
LGTM nice to see it come together! just to clarify something though: the language in the notes says something about serializing twice but calculateObjectSize doesn't serialize but it does recursively run over the whole object which is probably the source of the bad perf esp for very large insertMany inputs.
There are probably perf improvements that can be made to calc object size.. but that's a diff story..
|
@nbbeeken yep, thanks! Changed the wording to "processed" and "walked", calcSize is not serializing indeed, only traverse everything. |
PavelSafronov
left a comment
There was a problem hiding this comment.
I think this introduces a breaking change where we end up ignoring BulkWriteOptions passed to execute.
| const bson = this.s.bsonOptions; | ||
| buffer = BSON.serialize(document, { | ||
| checkKeys: this.s.checkKeys, | ||
| ignoreUndefined: bson.ignoreUndefined, |
There was a problem hiding this comment.
We are serializing documents here according to current bson.ignoreUndefined, but it's possible for the user to pass different options to execute, so this would be a behavior change.
Example:
const bulk = collection.initializeOrderedBulkOp();
bulk.insert({ _id: 1, a: undefined });
await bulk.execute({ ignoreUndefined: true });
We're now going to serialize on insert and use ignoreUndefined: false, so we'll end up ignoring the options on execute.
There was a problem hiding this comment.
That's awesome, thanks! I wasn't aware that user can supply ignoreUndefined in so so many places.
Addressed in latest commit.
PavelSafronov
left a comment
There was a problem hiding this comment.
Looks good! Approving and moving to team review.
Description
Summary of Changes
collection.insertMany()/bulkWrite()processed every document twice - once viaBSON.calculateObjectSize(for the max-size check and byte-based batch splitting) and again when the command was built. This change serializes each operation once inaddToOperationsList, stores the buffer on theBatch, and reuses it as anOP_MSGdocument sequence (payload type 1) in the insert/update/delete command builders - reusing the mechanism added forMongoClient.bulkWritein NODE-6325 (#4201).The array (payload type 0) path is retained under auto-encryption and for non-bulk callers. Command monitoring's extractCommand now reconstructs documents/updates/deletes sequences back into arrays. Behavior is preserved, including the default
ignoreUndefined: falsesemantics.Notes for Reviewers
Whooping 60% boost in large doc bulkwrite!
Release Highlight
Bulk writes serialize each document only once
insertManyandbulkWritepreviously processed each document twice - once to measure its size for batch splitting (a full recursive walk viacalculateObjectSize) and again to serialize it into the command sent to the server. Documents are now serialized a single time and the resulting bytes are reused for both, decreasing the BSON-encoding CPU spent on bulk writes and reducing event-loop blocking during large batches. The improvement is most noticeable with high document counts and documents that have many fields.Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript