CSHARP-5348: Avoid allocations for Bson*Context #1791

BorisDog · 2025-10-13T22:26:34Z

Convert BsonBinaryWriterContext and BsonBinaryReaderContext to structs, to reduces allocations on a hot path.

BorisDog · 2025-10-13T22:27:08Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

            _bsonStream = (stream as BsonStream) ?? new BsonStreamAdapter(stream);

-            _context = null;
+            _context = new(ContextType.TopLevel, 0);


Use TopLevel instead of null

BorisDog · 2025-10-14T01:06:16Z

tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs

    public class BsonBinaryReaderTests
    {
+        [Fact]
+        public void Bookmarks_should_work()


Added bookmarks test for better coverage of BsonBinaryReader.

BorisDog · 2025-10-14T01:06:19Z

tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs

-                GuidRepresentation.Unspecified)] GuidRepresentation guidRepresentation)
+        [InlineData(BsonBinarySubType.UuidStandard)]
+        [InlineData(BsonBinarySubType.UuidLegacy)]
+        public void ReadBinaryData_should_read_correct_subtype(BsonBinarySubType subType)


Unrelated cleanup.
GuidRepresentation wasn't used in ReadBinaryData_subtype_3_should_use_GuidRepresentation_from_settings and ReadBinaryData_subtype_4_should_use_GuidRepresentation_Standard tests. Both tests combined into single ReadBinaryData_should_read_correct_subtype test.

rstam · 2025-10-15T21:32:42Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

 #pragma warning restore CA2213 // Disposable never disposed
        private readonly BsonStream _bsonStream;
        private BsonBinaryReaderContext _context;
+        private readonly Lazy<Stack<BsonBinaryReaderContext>> _contexts = new(() => new());


We've traded allocations of contexts for whatever allocations the stack class does.

Consider using a non-default initial capacity, probably around 4 (maybe 8), otherwise the first few calls to Push will result in memory allocations.

Lazy requires an allocation also. Maybe just go ahead and allocate the Stack?

Actually maybe multiple allocations. I think the factory lambda is an allocation as well.

private readonly Stack<BsonBinaryReaderContext> _contextStack = new(4);

That's a good suggestion, done.

rstam · 2025-10-16T18:26:04Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

-            return new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _bsonStream.Position);
-        }
+        public override BsonReaderBookmark GetBookmark() =>
+            new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contexts.Value.ToArray(), _bsonStream.Position);


Let's encapsulate all logic about how to restore the context stack into the bookmark, so here we can just pass the _contextStack and let the bookmark call ToArray (or whatever it wants to do).

new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contextStack, _bsonStream.Position);

rstam · 2025-10-16T18:26:46Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs


-            _context = _context.PopContext(_bsonStream.Position);
+            _context = PopContext();
+


Let's encapsulate all side effects of popping the context in the PopContext method (see below).

PopContext();

Good idea, done.

rstam · 2025-10-16T18:27:02Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

            }

-            _context = _context.PopContext(_bsonStream.Position);
+            _context = PopContext();


PopContext();

rstam · 2025-10-16T18:27:12Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

            if (_context.ContextType == ContextType.JavaScriptWithScope)
            {
-                _context = _context.PopContext(_bsonStream.Position); // JavaScriptWithScope
+                _context = PopContext(); // JavaScriptWithScope


PopContext(); // JavaScriptWithScope

rstam · 2025-10-16T18:37:28Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

                {
                    BackpatchSize(); // size of the JavaScript with scope value
-                    _context = _context.ParentContext;
+                    _context = _contexts.Value.Pop();


PopContext();

rstam · 2025-10-16T18:37:56Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

-            _context = new BsonBinaryWriterContext(_context, ContextType.JavaScriptWithScope, _bsonStream.Position);
+
+            _contexts.Value.Push(_context);
+            _context = new(ContextType.JavaScriptWithScope, _bsonStream.Position);


Single line instead of two:

PushContext(new(ContextType.JavaScriptWithScope, _bsonStream.Position));

rstam · 2025-10-16T18:38:10Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

                {
                    BackpatchSize(); // size of the JavaScript with scope value
-                    _context = _context.ParentContext;
+                    _context = _contexts.Value.Pop();


PopContext();

rstam · 2025-10-16T18:38:22Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

-            _context = new BsonBinaryWriterContext(_context, ContextType.Array, _bsonStream.Position);
+
+            _contexts.Value.Push(_context);
+            _context = new(ContextType.Array, _bsonStream.Position);


PushContext(new(ContextType.Array, _bsonStream.Position));

rstam · 2025-10-16T18:38:43Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

            var contextType = (State == BsonWriterState.ScopeDocument) ? ContextType.ScopeDocument : ContextType.Document;
-            _context = new BsonBinaryWriterContext(_context, contextType, _bsonStream.Position);
+            _contexts.Value.Push(_context);
+            _context = new(contextType, _bsonStream.Position);


PushContext(new(contextType, _bsonStream.Position));

Add new helper methods:

private void PopContext() { _context = _contextStack.Pop(); } private void PushContext(BsonBinaryWriterContext context) { _contextStack.Push(_context); _context = context; }

Copilot

Pull Request Overview

This PR converts BsonBinaryReaderContext and BsonBinaryWriterContext from reference types (classes) to value types (structs) to reduce heap allocations on hot paths during BSON serialization/deserialization.

Key changes:

Converting context classes to structs with primary constructors
Implementing stack-based context management to replace parent-child context chains
Refactoring bookmark restoration logic to work with value-type contexts

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/MongoDB.Bson/IO/BsonBinaryWriterContext.cs	Converts class to struct with primary constructor and init-only properties
src/MongoDB.Bson/IO/BsonBinaryWriter.cs	Adds context stack and helper methods to manage struct-based contexts
src/MongoDB.Bson/IO/BsonBinaryReaderContext.cs	Converts class to struct, removes Clone/PopContext methods, renames properties
src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs	Updates bookmark to store context array instead of cloning context chain
src/MongoDB.Bson/IO/BsonBinaryReader.cs	Adds context stack, updates context management, refactors dotted name generation
tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs	Adds bookmark test and simplifies binary data subtype tests

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-17T23:22:50Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

+                throw new FormatException($"Expected size to be {_context.Size}, not {actualSize}.");
+            }
+
+            _context =_contexts.Pop();


Missing space after '=' operator.

Suggested change

_context =_contexts.Pop();

_context = _contexts.Pop();

Copilot · 2025-10-17T23:22:51Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

-            {
-                var indexElementName = context.CurrentArrayIndex.ToString(NumberFormatInfo.InvariantInfo);
-                return GenerateDottedElementName(context.ParentContext, indexElementName + "." + elementName);
+                return GenerateDottedElementName(contexts,  nextIndex, (context.ElementName ?? "?") + "." + elementName);


Extra space after comma in method calls.

Suggested change

return GenerateDottedElementName(contexts, nextIndex, (context.ElementName ?? "?") + "." + elementName);

return GenerateDottedElementName(contexts, nextIndex, (context.ElementName ?? "?") + "." + elementName);

Copilot · 2025-10-17T23:22:51Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

            {
-                return GenerateDottedElementName(context.ParentContext, "?." + elementName);
+                var indexElementName = context.ArrayIndex.ToString(NumberFormatInfo.InvariantInfo);
+                return GenerateDottedElementName(contexts,  nextIndex, indexElementName + "." + elementName);


Extra space after comma in method calls.

Suggested change

return GenerateDottedElementName(contexts, nextIndex, indexElementName + "." + elementName);

return GenerateDottedElementName(contexts, nextIndex, indexElementName + "." + elementName);

BorisDog · 2025-10-17T17:07:17Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

 #pragma warning restore CA2213 // Disposable never disposed
        private readonly BsonStream _bsonStream;
        private BsonBinaryReaderContext _context;
+        private readonly Lazy<Stack<BsonBinaryReaderContext>> _contexts = new(() => new());


That's a good suggestion, done.

BorisDog · 2025-10-17T17:13:52Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

-            return new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _bsonStream.Position);
-        }
+        public override BsonReaderBookmark GetBookmark() =>
+            new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contexts.Value.ToArray(), _bsonStream.Position);


BorisDog · 2025-10-17T17:18:03Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs


-            _context = _context.PopContext(_bsonStream.Position);
+            _context = PopContext();
+


Good idea, done.

BorisDog · 2025-10-17T17:18:08Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

            }

-            _context = _context.PopContext(_bsonStream.Position);
+            _context = PopContext();


BorisDog · 2025-10-17T18:28:37Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

            if (_context.ContextType == ContextType.JavaScriptWithScope)
            {
-                _context = _context.PopContext(_bsonStream.Position); // JavaScriptWithScope
+                _context = PopContext(); // JavaScriptWithScope


BorisDog · 2025-10-17T23:19:49Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

                {
                    BackpatchSize(); // size of the JavaScript with scope value
-                    _context = _context.ParentContext;
+                    _context = _contexts.Value.Pop();


BorisDog · 2025-10-17T23:19:51Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs


-            _context = _context.ParentContext;
-            if (_context == null)
+            _context = _contexts.Value.Pop();


BorisDog · 2025-10-17T23:19:54Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

            BackpatchSize(); // size of document

-            _context = _context.ParentContext;
+            _context = _contexts.Value.Pop();


BorisDog · 2025-10-17T23:20:02Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

 #pragma warning restore CA2213 // Disposable never disposed
        private readonly BsonStream _bsonStream;
        private BsonBinaryWriterContext _context;
+        private readonly Lazy<Stack<BsonBinaryWriterContext>> _contexts = new(() => new());


BorisDog · 2025-10-17T23:22:51Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

                    Close();
                }
-                catch { } // ignore exceptions
+                catch { /* ignore exceptions */ }


Make Rider happier.

rstam

A few minor comments. Would be willing to LGTM as is if you don't want to make the suggested changes.

rstam · 2025-10-20T19:17:03Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

 #pragma warning restore CA2213 // Disposable never disposed
        private readonly BsonStream _bsonStream;
        private BsonBinaryReaderContext _context;
+        private readonly Stack<BsonBinaryReaderContext> _contexts = new(4);


You didn't like the suggestion to rename this field to _contextStack?

rstam · 2025-10-20T19:18:59Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

 #pragma warning restore CA2213 // Disposable never disposed
        private readonly BsonStream _bsonStream;
        private BsonBinaryWriterContext _context;
+        private readonly Stack<BsonBinaryWriterContext> _contexts = new(4);


You didn't like the suggestion to rename this field to _contextStack?

rstam · 2025-10-20T19:19:45Z

src/MongoDB.Bson/IO/BsonBinaryWriter.cs

            }
        }
+
+        private void PopContext()


Do we want these in alphabetical order?

rstam · 2025-10-20T19:21:37Z

src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs

+        BsonType currentBsonType,
+        string currentName,
+        BsonBinaryReaderContext currentContext,
+        Stack<BsonBinaryReaderContext> contextsStack,


I think contextStack sounds more grammatical than contextsStack.

Thanks, done.

rstam · 2025-10-20T19:21:59Z

src/MongoDB.Bson/IO/BsonBinaryReader.cs

+using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Linq;


Unused using.

Good catch, done.

rstam

LGTM

CSHARP-5348: Avoid allocations for Bson*Context

78e106e

BorisDog requested a review from rstam October 13, 2025 22:26

BorisDog requested a review from a team as a code owner October 13, 2025 22:26

BorisDog added the improvement label Oct 13, 2025

BorisDog commented Oct 14, 2025

View reviewed changes

rstam requested changes Oct 16, 2025

View reviewed changes

- PR comments

51ae949

Copilot AI review requested due to automatic review settings October 17, 2025 23:22

Copilot AI reviewed Oct 17, 2025

View reviewed changes

BorisDog commented Oct 17, 2025

View reviewed changes

BorisDog requested a review from rstam October 17, 2025 23:23

rstam requested changes Oct 20, 2025

View reviewed changes

- PR comments

c18f0dc

BorisDog requested a review from rstam October 22, 2025 19:37

rstam approved these changes Oct 23, 2025

View reviewed changes


		_context = _context.PopContext(_bsonStream.Position);
		_context = PopContext();

	return GenerateDottedElementName(contexts, nextIndex, (context.ElementName ?? "?") + "." + elementName);
	return GenerateDottedElementName(contexts, nextIndex, (context.ElementName ?? "?") + "." + elementName);

CSHARP-5348: Avoid allocations for Bson*Context #1791

Are you sure you want to change the base?

CSHARP-5348: Avoid allocations for Bson*Context #1791

Uh oh!

Conversation

BorisDog commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog commented Oct 13, 2025 •

edited

Loading