From 78e106e46fe1d599fc87dda6966babfded06051d Mon Sep 17 00:00:00 2001 From: BorisDog Date: Mon, 13 Oct 2025 15:05:44 -0700 Subject: [PATCH 1/3] CSHARP-5348: Avoid allocations for Bson*Context --- src/MongoDB.Bson/IO/BsonBinaryReader.cs | 98 ++++++++---- .../IO/BsonBinaryReaderBookmark.cs | 54 +++---- .../IO/BsonBinaryReaderContext.cs | 78 ++-------- src/MongoDB.Bson/IO/BsonBinaryWriter.cs | 28 ++-- .../IO/BsonBinaryWriterContext.cs | 47 +----- .../IO/BsonBinaryReaderTests.cs | 142 +++++++++++------- 6 files changed, 208 insertions(+), 239 deletions(-) diff --git a/src/MongoDB.Bson/IO/BsonBinaryReader.cs b/src/MongoDB.Bson/IO/BsonBinaryReader.cs index 3bf194cd425..2912424033b 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReader.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReader.cs @@ -14,8 +14,10 @@ */ using System; +using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Linq; namespace MongoDB.Bson.IO { @@ -30,6 +32,7 @@ public class BsonBinaryReader : BsonReader #pragma warning restore CA2213 // Disposable never disposed private readonly BsonStream _bsonStream; private BsonBinaryReaderContext _context; + private readonly Lazy> _contexts = new(() => new()); // constructors /// @@ -61,7 +64,7 @@ public BsonBinaryReader(Stream stream, BsonBinaryReaderSettings settings) _baseStream = stream; _bsonStream = (stream as BsonStream) ?? new BsonStreamAdapter(stream); - _context = new BsonBinaryReaderContext(null, ContextType.TopLevel, 0, 0); + _context = new BsonBinaryReaderContext(ContextType.TopLevel, 0, 0); } // public properties @@ -109,10 +112,8 @@ public override void Close() /// Gets a bookmark to the reader's current position and state. /// /// A bookmark. - public override BsonReaderBookmark GetBookmark() - { - return new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _bsonStream.Position); - } + public override BsonReaderBookmark GetBookmark() => + new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contexts.Value.ToArray(), _bsonStream.Position); /// /// Determines whether this reader is at end of file. @@ -201,12 +202,11 @@ public override BsonType ReadBsonType() if (_context.ContextType == ContextType.Array) { - _context.CurrentArrayIndex++; + _context.ArrayIndex++; } try { - CurrentBsonType = _bsonStream.ReadBsonType(); } catch (FormatException ex) @@ -342,7 +342,8 @@ public override void ReadEndArray() ThrowInvalidState("ReadEndArray", BsonReaderState.EndOfArray); } - _context = _context.PopContext(_bsonStream.Position); + _context = PopContext(); + switch (_context.ContextType) { case ContextType.Array: State = BsonReaderState.Type; break; @@ -371,10 +372,10 @@ public override void ReadEndDocument() ThrowInvalidState("ReadEndDocument", BsonReaderState.EndOfDocument); } - _context = _context.PopContext(_bsonStream.Position); + _context = PopContext(); if (_context.ContextType == ContextType.JavaScriptWithScope) { - _context = _context.PopContext(_bsonStream.Position); // JavaScriptWithScope + _context = PopContext(); // JavaScriptWithScope } switch (_context.ContextType) { @@ -432,7 +433,10 @@ public override string ReadJavaScriptWithScope() var startPosition = _bsonStream.Position; // position of size field var size = ReadSize(); - _context = new BsonBinaryReaderContext(_context, ContextType.JavaScriptWithScope, startPosition, size); + + PushContext(_context); + _context = new BsonBinaryReaderContext(ContextType.JavaScriptWithScope, startPosition, size); + var code = _bsonStream.ReadString(Settings.Encoding); State = BsonReaderState.ScopeDocument; @@ -486,7 +490,7 @@ public override string ReadName(INameDecoder nameDecoder) if (_context.ContextType == ContextType.Document) { - _context.CurrentElementName = CurrentName; + _context.ElementName = CurrentName; } return CurrentName; @@ -553,7 +557,7 @@ public override IByteBuffer ReadRawBsonDocument() if (_context.ContextType == ContextType.JavaScriptWithScope) { - _context = _context.PopContext(_bsonStream.Position); // JavaScriptWithScope + _context = PopContext(); // JavaScriptWithScope } switch (_context.ContextType) { @@ -590,7 +594,10 @@ public override void ReadStartArray() var startPosition = _bsonStream.Position; // position of size field var size = ReadSize(); - _context = new BsonBinaryReaderContext(_context, ContextType.Array, startPosition, size); + + PushContext(_context); + _context = new(ContextType.Array, startPosition, size); + State = BsonReaderState.Type; } @@ -605,7 +612,8 @@ public override void ReadStartDocument() var contextType = (State == BsonReaderState.ScopeDocument) ? ContextType.ScopeDocument : ContextType.Document; var startPosition = _bsonStream.Position; // position of size field var size = ReadSize(); - _context = new BsonBinaryReaderContext(_context, contextType, startPosition, size); + PushContext(_context); + _context = new(contextType, startPosition, size); State = BsonReaderState.Type; } @@ -665,7 +673,15 @@ public override void ReturnToBookmark(BsonReaderBookmark bookmark) State = binaryReaderBookmark.State; CurrentBsonType = binaryReaderBookmark.CurrentBsonType; CurrentName = binaryReaderBookmark.CurrentName; - _context = binaryReaderBookmark.CloneContext(); + + _context = binaryReaderBookmark.CurrentContext; + + _contexts.Value.Clear(); + foreach (var context in binaryReaderBookmark.ContextsStack.Reverse()) + { + _contexts.Value.Push(context); + } + _bsonStream.Position = binaryReaderBookmark.Position; } @@ -686,7 +702,7 @@ public override void SkipName() if (_context.ContextType == ContextType.Document) { - _context.CurrentElementName = CurrentName; + _context.ElementName = CurrentName; } } @@ -767,35 +783,41 @@ private string GenerateDottedElementName() } else if (_context.ContextType == ContextType.Array) { - elementName = _context.CurrentArrayIndex.ToString(NumberFormatInfo.InvariantInfo); + elementName = _context.ArrayIndex.ToString(NumberFormatInfo.InvariantInfo); } else { elementName = "?"; } - return GenerateDottedElementName(_context.ParentContext, elementName); + return GenerateDottedElementName(_contexts.Value.ToArray(), 0, elementName); } - private string GenerateDottedElementName(BsonBinaryReaderContext context, string elementName) + private string GenerateDottedElementName(BsonBinaryReaderContext[] contexts, int currentContextIndex, string elementName) { + if (currentContextIndex >= contexts.Length) + return elementName; + + var context = contexts[currentContextIndex]; + var nextIndex = currentContextIndex + 1; + if (context.ContextType == ContextType.Document) { - return GenerateDottedElementName(context.ParentContext, (context.CurrentElementName ?? "?") + "." + elementName); + return GenerateDottedElementName(contexts, nextIndex, (context.ElementName ?? "?") + "." + elementName); } - else if (context.ContextType == ContextType.Array) - { - var indexElementName = context.CurrentArrayIndex.ToString(NumberFormatInfo.InvariantInfo); - return GenerateDottedElementName(context.ParentContext, indexElementName + "." + elementName); - } - else if (context.ParentContext != null) + + if (context.ContextType == ContextType.Array) { - return GenerateDottedElementName(context.ParentContext, "?." + elementName); + var indexElementName = context.ArrayIndex.ToString(NumberFormatInfo.InvariantInfo); + return GenerateDottedElementName(contexts, nextIndex, indexElementName + "." + elementName); } - else + + if (nextIndex < contexts.Length) { - return elementName; + return GenerateDottedElementName(contexts, nextIndex, "?." + elementName); } + + return elementName; } private BsonReaderState GetNextState() @@ -828,5 +850,21 @@ private int ReadSize() } return size; } + + private BsonBinaryReaderContext PopContext() + { + var actualSize = _bsonStream.Position - _context.StartPosition; + if (actualSize != _context.Size) + { + throw new FormatException($"Expected size to be {_context.Size}, not {actualSize}."); + } + + return _contexts.Value.Pop(); + } + + private void PushContext(BsonBinaryReaderContext context) + { + _contexts.Value.Push(context); + } } } diff --git a/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs b/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs index a449063236f..210d3258372 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs @@ -13,40 +13,28 @@ * limitations under the License. */ -namespace MongoDB.Bson.IO +namespace MongoDB.Bson.IO; + +/// +/// Represents a bookmark that can be used to return a reader to the current position and state. +/// +public class BsonBinaryReaderBookmark : BsonReaderBookmark { - /// - /// Represents a bookmark that can be used to return a reader to the current position and state. - /// - public class BsonBinaryReaderBookmark : BsonReaderBookmark + internal BsonBinaryReaderBookmark( + BsonReaderState state, + BsonType currentBsonType, + string currentName, + BsonBinaryReaderContext currentContext, + BsonBinaryReaderContext[] contextsStack, + long position) + : base(state, currentBsonType, currentName) { - // private fields - private BsonBinaryReaderContext _context; - private long _position; - - // constructors - internal BsonBinaryReaderBookmark( - BsonReaderState state, - BsonType currentBsonType, - string currentName, - BsonBinaryReaderContext context, - long position) - : base(state, currentBsonType, currentName) - { - _context = context.Clone(); - _position = position; - } - - // internal properties - internal long Position - { - get { return _position; } - } - - // internal methods - internal BsonBinaryReaderContext CloneContext() - { - return _context.Clone(); - } + CurrentContext = currentContext; + ContextsStack = contextsStack; + Position = position; } + + internal BsonBinaryReaderContext CurrentContext { get; } + internal BsonBinaryReaderContext[] ContextsStack { get; } + internal long Position { get; } } diff --git a/src/MongoDB.Bson/IO/BsonBinaryReaderContext.cs b/src/MongoDB.Bson/IO/BsonBinaryReaderContext.cs index c4e18381dda..41ccd9c7b5f 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReaderContext.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReaderContext.cs @@ -13,75 +13,17 @@ * limitations under the License. */ -using System; +namespace MongoDB.Bson.IO; -namespace MongoDB.Bson.IO +internal struct BsonBinaryReaderContext( + ContextType contextType, + long startPosition, + long size) { - internal class BsonBinaryReaderContext - { - // private fields - private readonly BsonBinaryReaderContext _parentContext; - private readonly ContextType _contextType; - private readonly long _startPosition; - private readonly long _size; - private string _currentElementName; - private int _currentArrayIndex = -1; + public ContextType ContextType { get; } = contextType; + public long StartPosition { get; } = startPosition; + public long Size { get; } = size; - // constructors - internal BsonBinaryReaderContext( - BsonBinaryReaderContext parentContext, - ContextType contextType, - long startPosition, - long size) - { - _parentContext = parentContext; - _contextType = contextType; - _startPosition = startPosition; - _size = size; - } - - // public properties - public ContextType ContextType - { - get { return _contextType; } - } - - public int CurrentArrayIndex - { - get { return _currentArrayIndex; } - set { _currentArrayIndex = value; } - } - - public string CurrentElementName - { - get { return _currentElementName; } - set { _currentElementName = value; } - } - - public BsonBinaryReaderContext ParentContext - { - get { return _parentContext; } - } - - // public methods - /// - /// Creates a clone of the context. - /// - /// A clone of the context. - public BsonBinaryReaderContext Clone() - { - return new BsonBinaryReaderContext(_parentContext, _contextType, _startPosition, _size); - } - - public BsonBinaryReaderContext PopContext(long position) - { - var actualSize = position - _startPosition; - if (actualSize != _size) - { - var message = string.Format("Expected size to be {0}, not {1}.", _size, actualSize); - throw new FormatException(message); - } - return _parentContext; - } - } + public int ArrayIndex { get; set; } = -1; + public string ElementName { get; set; } } diff --git a/src/MongoDB.Bson/IO/BsonBinaryWriter.cs b/src/MongoDB.Bson/IO/BsonBinaryWriter.cs index 42430ef4eb4..adaaee65112 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryWriter.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryWriter.cs @@ -30,6 +30,7 @@ public class BsonBinaryWriter : BsonWriter #pragma warning restore CA2213 // Disposable never disposed private readonly BsonStream _bsonStream; private BsonBinaryWriterContext _context; + private readonly Lazy> _contexts = new(() => new()); // constructors /// @@ -61,7 +62,7 @@ public BsonBinaryWriter(Stream stream, BsonBinaryWriterSettings settings) _baseStream = stream; _bsonStream = (stream as BsonStream) ?? new BsonStreamAdapter(stream); - _context = null; + _context = new(ContextType.TopLevel, 0); State = BsonWriterState.Initial; } @@ -115,7 +116,7 @@ public override void Close() { Flush(); } - _context = null; + _context = default; State = BsonWriterState.Closed; } } @@ -290,7 +291,7 @@ public override void WriteEndArray() _bsonStream.WriteByte(0); BackpatchSize(); // size of document - _context = _context.ParentContext; + _context = _contexts.Value.Pop(); State = GetNextState(); } @@ -313,8 +314,8 @@ public override void WriteEndDocument() _bsonStream.WriteByte(0); BackpatchSize(); // size of document - _context = _context.ParentContext; - if (_context == null) + _context = _contexts.Value.Pop(); + if (_context.ContextType == ContextType.TopLevel) { State = BsonWriterState.Done; } @@ -323,7 +324,7 @@ public override void WriteEndDocument() if (_context.ContextType == ContextType.JavaScriptWithScope) { BackpatchSize(); // size of the JavaScript with scope value - _context = _context.ParentContext; + _context = _contexts.Value.Pop(); } State = GetNextState(); } @@ -400,7 +401,9 @@ public override void WriteJavaScriptWithScope(string code) _bsonStream.WriteBsonType(BsonType.JavaScriptWithScope); WriteNameHelper(); - _context = new BsonBinaryWriterContext(_context, ContextType.JavaScriptWithScope, _bsonStream.Position); + + _contexts.Value.Push(_context); + _context = new(ContextType.JavaScriptWithScope, _bsonStream.Position); _bsonStream.WriteInt32(0); // reserve space for size of JavaScript with scope value _bsonStream.WriteString(code, Settings.Encoding); @@ -515,7 +518,7 @@ public override void WriteRawBsonDocument(IByteBuffer slice) } _bsonStream.WriteSlice(slice); // assumes byteBuffer is a valid raw BSON document - if (_context == null) + if (_context.ContextType == ContextType.TopLevel) { State = BsonWriterState.Done; } @@ -524,7 +527,7 @@ public override void WriteRawBsonDocument(IByteBuffer slice) if (_context.ContextType == ContextType.JavaScriptWithScope) { BackpatchSize(); // size of the JavaScript with scope value - _context = _context.ParentContext; + _context = _contexts.Value.Pop(); } State = GetNextState(); } @@ -564,7 +567,9 @@ public override void WriteStartArray() base.WriteStartArray(); _bsonStream.WriteBsonType(BsonType.Array); WriteNameHelper(); - _context = new BsonBinaryWriterContext(_context, ContextType.Array, _bsonStream.Position); + + _contexts.Value.Push(_context); + _context = new(ContextType.Array, _bsonStream.Position); _bsonStream.WriteInt32(0); // reserve space for size State = BsonWriterState.Value; @@ -588,7 +593,8 @@ public override void WriteStartDocument() WriteNameHelper(); } 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); _bsonStream.WriteInt32(0); // reserve space for size State = BsonWriterState.Name; diff --git a/src/MongoDB.Bson/IO/BsonBinaryWriterContext.cs b/src/MongoDB.Bson/IO/BsonBinaryWriterContext.cs index fb791ab4fc3..a5ec9bf2f64 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryWriterContext.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryWriterContext.cs @@ -13,47 +13,12 @@ * limitations under the License. */ -namespace MongoDB.Bson.IO -{ - internal class BsonBinaryWriterContext - { - // private fields - private BsonBinaryWriterContext _parentContext; - private ContextType _contextType; - private long _startPosition; - private int _index; // used when contextType is Array - - // constructors - internal BsonBinaryWriterContext( - BsonBinaryWriterContext parentContext, - ContextType contextType, - long startPosition) - { - _parentContext = parentContext; - _contextType = contextType; - _startPosition = startPosition; - } - - // internal properties - internal BsonBinaryWriterContext ParentContext - { - get { return _parentContext; } - } +namespace MongoDB.Bson.IO; - internal ContextType ContextType - { - get { return _contextType; } - } - - internal long StartPosition - { - get { return _startPosition; } - } +internal struct BsonBinaryWriterContext(ContextType contextType, long startPosition) +{ + public ContextType ContextType { get; } = contextType; + public long StartPosition { get; } = startPosition; - internal int Index - { - get { return _index; } - set { _index = value; } - } - } + public int Index { get; set; } } diff --git a/tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs b/tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs index 90c33da9158..c13ff294cbc 100644 --- a/tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs +++ b/tests/MongoDB.Bson.Tests/IO/BsonBinaryReaderTests.cs @@ -27,6 +27,73 @@ namespace MongoDB.Bson.Tests.IO { public class BsonBinaryReaderTests { + [Fact] + public void Bookmarks_should_work() + { + var document = new BsonDocument { { "x", 1 }, { "y", 2 }, { "z", new BsonDocument { { "a", "a"}, { "b", "b" } } } }; + + using var memoryStream = new MemoryStream(document.ToBson()); + using var bsonReader = new BsonBinaryReader(memoryStream); + + AssertRead(() => bsonReader.ReadBsonType(), BsonType.Document); + DoReaderAction(() => bsonReader.ReadStartDocument()); + + var bookmarkX = bsonReader.GetBookmark(); + AssertRead(() => bsonReader.ReadBsonType(), BsonType.Int32); + AssertRead(() => bsonReader.ReadName(), "x"); + AssertRead(() => bsonReader.ReadInt32(), 1); + + AssertRead(() => bsonReader.ReadBsonType(), BsonType.Int32); + AssertRead(() => bsonReader.ReadName(), "y"); + AssertRead(() => bsonReader.ReadInt32(), 2); + + AssertRead(() => bsonReader.ReadBsonType(), BsonType.Document); + AssertRead(() => bsonReader.ReadName(), "z"); + DoReaderAction(() => bsonReader.ReadStartDocument()); + + AssertRead(() => bsonReader.ReadBsonType(), BsonType.String); + AssertRead(() => bsonReader.ReadName(), "a"); + AssertRead(() => bsonReader.ReadString(), "a"); + + var bookmarkB = bsonReader.GetBookmark(); + AssertRead(() => bsonReader.ReadBsonType(), BsonType.String); + AssertRead(() => bsonReader.ReadName(), "b"); + AssertRead(() => bsonReader.ReadString(), "b"); + DoReaderAction(() => bsonReader.ReadEndDocument()); + + DoReaderAction(() => bsonReader.ReadEndDocument()); + bsonReader.State.Should().Be(BsonReaderState.Initial); + bsonReader.IsAtEndOfFile().Should().BeTrue(); + + bsonReader.ReturnToBookmark(bookmarkX); + AssertRead(() => bsonReader.ReadBsonType(), BsonType.Int32); + AssertRead(() => bsonReader.ReadName(), "x"); + AssertRead(() => bsonReader.ReadInt32(), 1); + + bsonReader.ReturnToBookmark(bookmarkB); + AssertRead(() => bsonReader.ReadBsonType(), BsonType.String); + AssertRead(() => bsonReader.ReadName(), "b"); + AssertRead(() => bsonReader.ReadString(), "b"); + DoReaderAction(() => bsonReader.ReadEndDocument()); + + // do everything twice returning to bookmark in between + void DoReaderAction(Action readerAction) + { + var bookmark = bsonReader.GetBookmark(); + readerAction(); + bsonReader.ReturnToBookmark(bookmark); + readerAction(); + } + + void AssertRead(Func reader, T expected) + { + var bookmark = bsonReader.GetBookmark(); + reader().Should().Be(expected); + bsonReader.ReturnToBookmark(bookmark); + reader().Should().Be(expected); + } + } + [Fact] public void BsonBinaryReader_should_support_reading_more_than_2GB() { @@ -344,64 +411,27 @@ public void TestReadRawBsonDocument() } [Theory] - [ParameterAttributeData] - public void ReadBinaryData_subtype_3_should_use_GuidRepresentation_from_settings( - [Values( - GuidRepresentation.CSharpLegacy, - GuidRepresentation.JavaLegacy, - GuidRepresentation.PythonLegacy, - GuidRepresentation.Unspecified)] GuidRepresentation guidRepresentation) + [InlineData(BsonBinarySubType.UuidStandard)] + [InlineData(BsonBinarySubType.UuidLegacy)] + public void ReadBinaryData_should_read_correct_subtype(BsonBinarySubType subType) { - var settings = new BsonBinaryReaderSettings(); - var bytes = new byte[] { 29, 0, 0, 0, 5, 120, 0, 16, 0, 0, 0, 3, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 0 }; - using (var stream = new MemoryStream(bytes)) - using (var reader = new BsonBinaryReader(stream, settings)) - { - reader.ReadStartDocument(); - var type = reader.ReadBsonType(); - var name = reader.ReadName(); - var binaryData = reader.ReadBinaryData(); - var endOfDocument = reader.ReadBsonType(); - reader.ReadEndDocument(); - - name.Should().Be("x"); - type.Should().Be(BsonType.Binary); - binaryData.SubType.Should().Be(BsonBinarySubType.UuidLegacy); - binaryData.Bytes.Should().Equal(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }); - endOfDocument.Should().Be(BsonType.EndOfDocument); - stream.Position.Should().Be(stream.Length); - } - } + var bytes = new byte[] { 29, 0, 0, 0, 5, 120, 0, 16, 0, 0, 0, (byte)subType, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 0 }; + using var stream = new MemoryStream(bytes); - [Theory] - [ParameterAttributeData] - public void ReadBinaryData_subtype_4_should_use_GuidRepresentation_Standard( - [Values( - GuidRepresentation.CSharpLegacy, - GuidRepresentation.JavaLegacy, - GuidRepresentation.PythonLegacy, - GuidRepresentation.Standard, - GuidRepresentation.Unspecified)] GuidRepresentation guidRepresentation) - { - var settings = new BsonBinaryReaderSettings(); - var bytes = new byte[] { 29, 0, 0, 0, 5, 120, 0, 16, 0, 0, 0, 4, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 0 }; - using (var stream = new MemoryStream(bytes)) - using (var reader = new BsonBinaryReader(stream, settings)) - { - reader.ReadStartDocument(); - var type = reader.ReadBsonType(); - var name = reader.ReadName(); - var binaryData = reader.ReadBinaryData(); - var endOfDocument = reader.ReadBsonType(); - reader.ReadEndDocument(); - - name.Should().Be("x"); - type.Should().Be(BsonType.Binary); - binaryData.SubType.Should().Be(BsonBinarySubType.UuidStandard); - binaryData.Bytes.Should().Equal(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }); - endOfDocument.Should().Be(BsonType.EndOfDocument); - stream.Position.Should().Be(stream.Length); - } + using var reader = new BsonBinaryReader(stream, new()); + reader.ReadStartDocument(); + var type = reader.ReadBsonType(); + var name = reader.ReadName(); + var binaryData = reader.ReadBinaryData(); + var endOfDocument = reader.ReadBsonType(); + reader.ReadEndDocument(); + + name.Should().Be("x"); + type.Should().Be(BsonType.Binary); + binaryData.SubType.Should().Be(subType); + binaryData.Bytes.Should().Equal(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16); + endOfDocument.Should().Be(BsonType.EndOfDocument); + stream.Position.Should().Be(stream.Length); } // private methods From 51ae9490c8c34d9d24ffbb75acd45a6f610b6023 Mon Sep 17 00:00:00 2001 From: BorisDog Date: Fri, 17 Oct 2025 16:21:56 -0700 Subject: [PATCH 2/3] - PR comments --- src/MongoDB.Bson/IO/BsonBinaryReader.cs | 47 ++++++++----------- .../IO/BsonBinaryReaderBookmark.cs | 29 +++++++++--- src/MongoDB.Bson/IO/BsonBinaryWriter.cs | 32 ++++++++----- 3 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/MongoDB.Bson/IO/BsonBinaryReader.cs b/src/MongoDB.Bson/IO/BsonBinaryReader.cs index 2912424033b..b59db397726 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReader.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReader.cs @@ -32,7 +32,7 @@ public class BsonBinaryReader : BsonReader #pragma warning restore CA2213 // Disposable never disposed private readonly BsonStream _bsonStream; private BsonBinaryReaderContext _context; - private readonly Lazy> _contexts = new(() => new()); + private readonly Stack _contexts = new(4); // constructors /// @@ -113,7 +113,7 @@ public override void Close() /// /// A bookmark. public override BsonReaderBookmark GetBookmark() => - new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contexts.Value.ToArray(), _bsonStream.Position); + new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contexts, _bsonStream.Position); /// /// Determines whether this reader is at end of file. @@ -342,7 +342,7 @@ public override void ReadEndArray() ThrowInvalidState("ReadEndArray", BsonReaderState.EndOfArray); } - _context = PopContext(); + PopContext(); switch (_context.ContextType) { @@ -372,10 +372,10 @@ public override void ReadEndDocument() ThrowInvalidState("ReadEndDocument", BsonReaderState.EndOfDocument); } - _context = PopContext(); + PopContext(); if (_context.ContextType == ContextType.JavaScriptWithScope) { - _context = PopContext(); // JavaScriptWithScope + PopContext(); // JavaScriptWithScope } switch (_context.ContextType) { @@ -434,8 +434,7 @@ public override string ReadJavaScriptWithScope() var startPosition = _bsonStream.Position; // position of size field var size = ReadSize(); - PushContext(_context); - _context = new BsonBinaryReaderContext(ContextType.JavaScriptWithScope, startPosition, size); + PushContext(new(ContextType.JavaScriptWithScope, startPosition, size)); var code = _bsonStream.ReadString(Settings.Encoding); @@ -557,7 +556,7 @@ public override IByteBuffer ReadRawBsonDocument() if (_context.ContextType == ContextType.JavaScriptWithScope) { - _context = PopContext(); // JavaScriptWithScope + PopContext(); // JavaScriptWithScope } switch (_context.ContextType) { @@ -595,8 +594,7 @@ public override void ReadStartArray() var startPosition = _bsonStream.Position; // position of size field var size = ReadSize(); - PushContext(_context); - _context = new(ContextType.Array, startPosition, size); + PushContext(new(ContextType.Array, startPosition, size)); State = BsonReaderState.Type; } @@ -612,8 +610,9 @@ public override void ReadStartDocument() var contextType = (State == BsonReaderState.ScopeDocument) ? ContextType.ScopeDocument : ContextType.Document; var startPosition = _bsonStream.Position; // position of size field var size = ReadSize(); - PushContext(_context); - _context = new(contextType, startPosition, size); + + PushContext(new(contextType, startPosition, size)); + State = BsonReaderState.Type; } @@ -670,18 +669,11 @@ public override void ReadUndefined() public override void ReturnToBookmark(BsonReaderBookmark bookmark) { var binaryReaderBookmark = (BsonBinaryReaderBookmark)bookmark; + State = binaryReaderBookmark.State; CurrentBsonType = binaryReaderBookmark.CurrentBsonType; CurrentName = binaryReaderBookmark.CurrentName; - - _context = binaryReaderBookmark.CurrentContext; - - _contexts.Value.Clear(); - foreach (var context in binaryReaderBookmark.ContextsStack.Reverse()) - { - _contexts.Value.Push(context); - } - + _context = binaryReaderBookmark.RestoreContext(_contexts); _bsonStream.Position = binaryReaderBookmark.Position; } @@ -761,7 +753,7 @@ protected override void Dispose(bool disposing) { Close(); } - catch { } // ignore exceptions + catch { /* ignore exceptions */ } } base.Dispose(disposing); } @@ -790,7 +782,7 @@ private string GenerateDottedElementName() elementName = "?"; } - return GenerateDottedElementName(_contexts.Value.ToArray(), 0, elementName); + return GenerateDottedElementName(_contexts.ToArray(), 0, elementName); } private string GenerateDottedElementName(BsonBinaryReaderContext[] contexts, int currentContextIndex, string elementName) @@ -851,7 +843,7 @@ private int ReadSize() return size; } - private BsonBinaryReaderContext PopContext() + private void PopContext() { var actualSize = _bsonStream.Position - _context.StartPosition; if (actualSize != _context.Size) @@ -859,12 +851,13 @@ private BsonBinaryReaderContext PopContext() throw new FormatException($"Expected size to be {_context.Size}, not {actualSize}."); } - return _contexts.Value.Pop(); + _context =_contexts.Pop(); } - private void PushContext(BsonBinaryReaderContext context) + private void PushContext(BsonBinaryReaderContext newContext) { - _contexts.Value.Push(context); + _contexts.Push(_context); + _context = newContext; } } } diff --git a/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs b/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs index 210d3258372..a0e3ff4ee7d 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs @@ -13,6 +13,7 @@ * limitations under the License. */ +using System.Collections.Generic; namespace MongoDB.Bson.IO; /// @@ -20,21 +21,35 @@ namespace MongoDB.Bson.IO; /// public class BsonBinaryReaderBookmark : BsonReaderBookmark { + private readonly BsonBinaryReaderContext _context; + private readonly BsonBinaryReaderContext[] _contextArray; + private readonly long _position; + internal BsonBinaryReaderBookmark( BsonReaderState state, BsonType currentBsonType, string currentName, BsonBinaryReaderContext currentContext, - BsonBinaryReaderContext[] contextsStack, + Stack contextsStack, long position) : base(state, currentBsonType, currentName) { - CurrentContext = currentContext; - ContextsStack = contextsStack; - Position = position; + _context = currentContext; + _contextArray = contextsStack.ToArray(); + _position = position; } - internal BsonBinaryReaderContext CurrentContext { get; } - internal BsonBinaryReaderContext[] ContextsStack { get; } - internal long Position { get; } + internal long Position => _position; + + internal BsonBinaryReaderContext RestoreContext(Stack contextStack) + { + contextStack.Clear(); + + for (var i = _contextArray.Length - 1; i >= 0; i--) + { + contextStack.Push(_contextArray[i]); + } + + return _context; + } } diff --git a/src/MongoDB.Bson/IO/BsonBinaryWriter.cs b/src/MongoDB.Bson/IO/BsonBinaryWriter.cs index adaaee65112..84e9ad9b08c 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryWriter.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryWriter.cs @@ -30,7 +30,7 @@ public class BsonBinaryWriter : BsonWriter #pragma warning restore CA2213 // Disposable never disposed private readonly BsonStream _bsonStream; private BsonBinaryWriterContext _context; - private readonly Lazy> _contexts = new(() => new()); + private readonly Stack _contexts = new(4); // constructors /// @@ -291,7 +291,7 @@ public override void WriteEndArray() _bsonStream.WriteByte(0); BackpatchSize(); // size of document - _context = _contexts.Value.Pop(); + PopContext(); State = GetNextState(); } @@ -314,7 +314,7 @@ public override void WriteEndDocument() _bsonStream.WriteByte(0); BackpatchSize(); // size of document - _context = _contexts.Value.Pop(); + PopContext(); if (_context.ContextType == ContextType.TopLevel) { State = BsonWriterState.Done; @@ -324,7 +324,7 @@ public override void WriteEndDocument() if (_context.ContextType == ContextType.JavaScriptWithScope) { BackpatchSize(); // size of the JavaScript with scope value - _context = _contexts.Value.Pop(); + PopContext(); } State = GetNextState(); } @@ -402,8 +402,7 @@ public override void WriteJavaScriptWithScope(string code) _bsonStream.WriteBsonType(BsonType.JavaScriptWithScope); WriteNameHelper(); - _contexts.Value.Push(_context); - _context = new(ContextType.JavaScriptWithScope, _bsonStream.Position); + PushContext(new(ContextType.JavaScriptWithScope, _bsonStream.Position)); _bsonStream.WriteInt32(0); // reserve space for size of JavaScript with scope value _bsonStream.WriteString(code, Settings.Encoding); @@ -527,7 +526,7 @@ public override void WriteRawBsonDocument(IByteBuffer slice) if (_context.ContextType == ContextType.JavaScriptWithScope) { BackpatchSize(); // size of the JavaScript with scope value - _context = _contexts.Value.Pop(); + PopContext(); } State = GetNextState(); } @@ -568,8 +567,7 @@ public override void WriteStartArray() _bsonStream.WriteBsonType(BsonType.Array); WriteNameHelper(); - _contexts.Value.Push(_context); - _context = new(ContextType.Array, _bsonStream.Position); + PushContext(new(ContextType.Array, _bsonStream.Position)); _bsonStream.WriteInt32(0); // reserve space for size State = BsonWriterState.Value; @@ -593,8 +591,7 @@ public override void WriteStartDocument() WriteNameHelper(); } var contextType = (State == BsonWriterState.ScopeDocument) ? ContextType.ScopeDocument : ContextType.Document; - _contexts.Value.Push(_context); - _context = new(contextType, _bsonStream.Position); + PushContext(new(contextType, _bsonStream.Position)); _bsonStream.WriteInt32(0); // reserve space for size State = BsonWriterState.Name; @@ -687,7 +684,7 @@ protected override void Dispose(bool disposing) { Close(); } - catch { } // ignore exceptions + catch { /* ignore exceptions */ } } base.Dispose(disposing); } @@ -733,5 +730,16 @@ private void WriteNameHelper() _bsonStream.WriteCString(Name); } } + + private void PopContext() + { + _context = _contexts.Pop(); + } + + private void PushContext(BsonBinaryWriterContext newContext) + { + _contexts.Push(_context); + _context = newContext; + } } } From c18f0dca6540e894197f5b66c4e2c50f399c0648 Mon Sep 17 00:00:00 2001 From: BorisDog Date: Wed, 22 Oct 2025 12:35:46 -0700 Subject: [PATCH 3/3] - PR comments --- src/MongoDB.Bson/IO/BsonBinaryReader.cs | 43 +++++++++---------- .../IO/BsonBinaryReaderBookmark.cs | 4 +- src/MongoDB.Bson/IO/BsonBinaryWriter.cs | 24 +++++------ 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/MongoDB.Bson/IO/BsonBinaryReader.cs b/src/MongoDB.Bson/IO/BsonBinaryReader.cs index b59db397726..1cc9f8f255d 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReader.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReader.cs @@ -17,7 +17,6 @@ using System.Collections.Generic; using System.Globalization; using System.IO; -using System.Linq; namespace MongoDB.Bson.IO { @@ -32,7 +31,7 @@ public class BsonBinaryReader : BsonReader #pragma warning restore CA2213 // Disposable never disposed private readonly BsonStream _bsonStream; private BsonBinaryReaderContext _context; - private readonly Stack _contexts = new(4); + private readonly Stack _contextStack = new(4); // constructors /// @@ -113,7 +112,7 @@ public override void Close() /// /// A bookmark. public override BsonReaderBookmark GetBookmark() => - new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contexts, _bsonStream.Position); + new BsonBinaryReaderBookmark(State, CurrentBsonType, CurrentName, _context, _contextStack, _bsonStream.Position); /// /// Determines whether this reader is at end of file. @@ -673,7 +672,7 @@ public override void ReturnToBookmark(BsonReaderBookmark bookmark) State = binaryReaderBookmark.State; CurrentBsonType = binaryReaderBookmark.CurrentBsonType; CurrentName = binaryReaderBookmark.CurrentName; - _context = binaryReaderBookmark.RestoreContext(_contexts); + _context = binaryReaderBookmark.RestoreContext(_contextStack); _bsonStream.Position = binaryReaderBookmark.Position; } @@ -782,7 +781,7 @@ private string GenerateDottedElementName() elementName = "?"; } - return GenerateDottedElementName(_contexts.ToArray(), 0, elementName); + return GenerateDottedElementName(_contextStack.ToArray(), 0, elementName); } private string GenerateDottedElementName(BsonBinaryReaderContext[] contexts, int currentContextIndex, string elementName) @@ -827,6 +826,23 @@ private BsonReaderState GetNextState() } } + private void PopContext() + { + var actualSize = _bsonStream.Position - _context.StartPosition; + if (actualSize != _context.Size) + { + throw new FormatException($"Expected size to be {_context.Size}, not {actualSize}."); + } + + _context =_contextStack.Pop(); + } + + private void PushContext(BsonBinaryReaderContext newContext) + { + _contextStack.Push(_context); + _context = newContext; + } + private int ReadSize() { int size = _bsonStream.ReadInt32(); @@ -842,22 +858,5 @@ private int ReadSize() } return size; } - - private void PopContext() - { - var actualSize = _bsonStream.Position - _context.StartPosition; - if (actualSize != _context.Size) - { - throw new FormatException($"Expected size to be {_context.Size}, not {actualSize}."); - } - - _context =_contexts.Pop(); - } - - private void PushContext(BsonBinaryReaderContext newContext) - { - _contexts.Push(_context); - _context = newContext; - } } } diff --git a/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs b/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs index a0e3ff4ee7d..80754719444 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryReaderBookmark.cs @@ -30,12 +30,12 @@ internal BsonBinaryReaderBookmark( BsonType currentBsonType, string currentName, BsonBinaryReaderContext currentContext, - Stack contextsStack, + Stack contextStack, long position) : base(state, currentBsonType, currentName) { _context = currentContext; - _contextArray = contextsStack.ToArray(); + _contextArray = contextStack.ToArray(); _position = position; } diff --git a/src/MongoDB.Bson/IO/BsonBinaryWriter.cs b/src/MongoDB.Bson/IO/BsonBinaryWriter.cs index 84e9ad9b08c..f70e4364d16 100644 --- a/src/MongoDB.Bson/IO/BsonBinaryWriter.cs +++ b/src/MongoDB.Bson/IO/BsonBinaryWriter.cs @@ -30,7 +30,7 @@ public class BsonBinaryWriter : BsonWriter #pragma warning restore CA2213 // Disposable never disposed private readonly BsonStream _bsonStream; private BsonBinaryWriterContext _context; - private readonly Stack _contexts = new(4); + private readonly Stack _contextStack = new(4); // constructors /// @@ -717,6 +717,17 @@ private BsonWriterState GetNextState() } } + private void PopContext() + { + _context = _contextStack.Pop(); + } + + private void PushContext(BsonBinaryWriterContext newContext) + { + _contextStack.Push(_context); + _context = newContext; + } + private void WriteNameHelper() { if (_context.ContextType == ContextType.Array) @@ -730,16 +741,5 @@ private void WriteNameHelper() _bsonStream.WriteCString(Name); } } - - private void PopContext() - { - _context = _contexts.Pop(); - } - - private void PushContext(BsonBinaryWriterContext newContext) - { - _contexts.Push(_context); - _context = newContext; - } } }