Skip to content

PYTHON-3419 Use memoryview to avoid byte copies when decoding larger RawBSONDocuments - #3003

Merged
NoahStapp merged 11 commits into
mongodb:mainfrom
NoahStapp:PYTHON-3419
Aug 26, 2026
Merged

PYTHON-3419 Use memoryview to avoid byte copies when decoding larger RawBSONDocuments#3003
NoahStapp merged 11 commits into
mongodb:mainfrom
NoahStapp:PYTHON-3419

Conversation

@NoahStapp

@NoahStapp NoahStapp commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

PYTHON-3419

Changes in this PR

RawBSONDocuments of size >= 4KB are decoded to memoryview slices of their buffer rather than bytes copies. This is a very significant performance improvement at no cost to documents smaller than the 4KB threshold:

inflate 100 10KB docs: ~17% faster
inflate 10 1MB docs: ~95% faster
decode 50 100KB docs: ~82% faster
additional memory used for 20 5MB docs: 86MB baseline vs ~0MB
inflate 1000 <= 4KB docs: flat

Test Plan

Added new unit tests in test/test_raw_bson_shared.py and new integration tests to test/(a)synchronous/test_raw_bson.py.

Checklist

Checklist for Author

  • Did you update the changelog (if necessary)?
  • Is there test coverage?
  • [ ] Is any followup work tracked in a JIRA ticket? If so, add link(s).

Checklist for Reviewer

  • Does the title of the PR reference a JIRA Ticket?
  • Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
  • Is all relevant documentation (README or docstring) updated?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes large RawBSONDocument decoding by retaining read-only buffer views instead of copying bytes.

Changes:

  • Adds zero-copy decoding for large raw BSON slices in Python and C.
  • Supports encoding, pickling, copying, and representing view-backed documents.
  • Adds shared unit tests, integration coverage, and documentation updates.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
bson/__init__.py Implements Python buffer slicing and encoding support.
bson/_cbsonmodule.c Implements C-extension zero-copy decoding.
bson/_cbsonmodule.h Tracks the owning decode buffer.
bson/codec_options.py Defines the 4096-byte threshold.
bson/json_util.py Documents python-bsonjs conversion requirements.
bson/raw_bson.py Supports view-backed raw data, serialization, and representation.
doc/changelog.rst Describes the performance and API changes.
test/test_raw_bson_shared.py Adds shared unit and regression coverage.
test/test_raw_bson.py Adds synchronous integration coverage.
test/asynchronous/test_raw_bson.py Adds asynchronous integration coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread doc/changelog.rst
Comment thread bson/raw_bson.py
Comment thread bson/__init__.py Outdated
Comment thread bson/codec_options.py
@NoahStapp
NoahStapp marked this pull request as ready for review August 21, 2026 16:01
@NoahStapp
NoahStapp requested a review from a team as a code owner August 21, 2026 16:01
@NoahStapp
NoahStapp requested a review from blink1073 August 21, 2026 16:01
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@blink1073

blink1073 commented Aug 22, 2026

Copy link
Copy Markdown
Member

decode_iter() does not have the improvements this PR adds to decode_all(), so this test fails:

def test_decode_iter_matches_decode_all_view_backing(self):
    opts = CodecOptions(document_class=RawBSONDocument)
    big = {"payload": b"x" * (_RAW_BSON_VIEW_THRESHOLD + 100)}
    stream = encode(big) * 3

    all_types = [type(d.raw) for d in decode_all(stream, opts)]
    iter_types = [type(d.raw) for d in decode_iter(stream, opts)]

    self.assertEqual(all_types, iter_types)

Also, elements_to_dict computes offset = string - PyBytes_AS_STRING(buffer_owner) and slices on it without checking that offset and offset + max land inside buffer_owner. Nothing enforces that today; it works only because all three call sites happen to derive string and buffer_owner from the same object. We should an assertion so a future mismatch fails loudly instead of silently returning truncated data:

Py_ssize_t offset = string - PyBytes_AS_STRING(buffer_owner);
assert(offset >= 0 && offset + max <= PyBytes_GET_SIZE(buffer_owner));
PyObject* top_view = PyMemoryView_FromObject(buffer_owner);

@NoahStapp

Copy link
Copy Markdown
Contributor Author

4.2 failures are expected until #2892 is merged.

@NoahStapp

NoahStapp commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

decode_iter() does not have the improvements this PR adds to decode_all(), so this test fails:

def test_decode_iter_matches_decode_all_view_backing(self):
    opts = CodecOptions(document_class=RawBSONDocument)
    big = {"payload": b"x" * (_RAW_BSON_VIEW_THRESHOLD + 100)}
    stream = encode(big) * 3

    all_types = [type(d.raw) for d in decode_all(stream, opts)]
    iter_types = [type(d.raw) for d in decode_iter(stream, opts)]

    self.assertEqual(all_types, iter_types)

Also, elements_to_dict computes offset = string - PyBytes_AS_STRING(buffer_owner) and slices on it without checking that offset and offset + max land inside buffer_owner. Nothing enforces that today; it works only because all three call sites happen to derive string and buffer_owner from the same object. We should an assertion so a future mismatch fails loudly instead of silently returning truncated data:

Py_ssize_t offset = string - PyBytes_AS_STRING(buffer_owner);
assert(offset >= 0 && offset + max <= PyBytes_GET_SIZE(buffer_owner));
PyObject* top_view = PyMemoryView_FromObject(buffer_owner);

I don't think we want to make the same changes to decode_iter for the following reasons:

  1. We don't currently implement a C-extension version of decode_iter. This implies that decode_iter is not intended for maximum performance and that users should use decode_all instead for such workloads, which is what I would expect. To get a speedup on the same scale as this PR does for decode_all, we'd need to add a C-extension implementation.
  2. The C _bson_to_dict changes in this PR already improve decode_iter's performance since both APIs call into it. The remaining difference is that decode_iter still copies each document as part of decoding. For a generator method, I think that copying is what we want to prevent the entire input buffer from being held in memory for the lifetime of the generator.
  3. We use decode_all within the driver, so a performance improvement there speeds up every operation. We don't use decode_iter at all, so I see it as less of a priority.
  4. I'm fine with decode_iter and decode_all having different behavior on large documents. If necessary, we can document that and specify more explicitly the intended use cases for each to reduce confusion.

Agreed with the safety assertion change!

@blink1073 blink1073 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NoahStapp
NoahStapp merged commit d6386c7 into mongodb:main Aug 26, 2026
88 of 90 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants