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);

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();

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; }

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


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

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BorisDog commented Oct 13, 2025 •

edited

Loading