Skip to content

CSHARP-5666: Remove GetBitArray allocations in BsonClassMapSerializer.DeserializeClass #1767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+274 −78
Open

CSHARP-5666: Remove GetBitArray allocations in BsonClassMapSerializer.DeserializeClass #1767

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7ff59a
CSHARP-5666: Remove GetBitArray allocations in BsonClassMapSerializer…
BorisDog Sep 1, 2025
ad91607
- Fix error message
BorisDog Sep 1, 2025
918ecac
- PR comments
BorisDog Sep 9, 2025
a913d58
- PR comments
BorisDog Sep 10, 2025
cc3bf13
- PR comments
BorisDog Sep 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 82 additions & 77 deletions src/MongoDB.Bson/Serialization/Serializers/BsonClassMapSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
*/

using System;
using System.Buffers;
using System.Collections.Generic;
using System.ComponentModel;
using System.Reflection;
using System.Runtime.CompilerServices;
using MongoDB.Bson.IO;
using MongoDB.Bson.Serialization.Conventions;
using MongoDB.Bson.Serialization.Serializers;
Expand Down Expand Up @@ -82,7 +84,7 @@ public override TClass Deserialize(BsonDeserializationContext context, BsonDeser
{
var bsonReader = context.Reader;

if (bsonReader.GetCurrentBsonType() == Bson.BsonType.Null)
if (bsonReader.GetCurrentBsonType() == BsonType.Null)
{
bsonReader.ReadNull();
return default(TClass);
Expand Down Expand Up @@ -149,7 +151,9 @@ public TClass DeserializeClass(BsonDeserializationContext context)
var discriminatorConvention = _classMap.GetDiscriminatorConvention();
var allMemberMaps = _classMap.AllMemberMaps;
var extraElementsMemberMapIndex = _classMap.ExtraElementsMemberMapIndex;
var memberMapBitArray = FastMemberMapHelper.GetBitArray(allMemberMaps.Count);

var (bitArrayLength, useStackAlloc) = FastMemberMapHelper.GetMembersBitArrayLength(_classMap.AllMemberMaps.Count);
using var bitArray = useStackAlloc ? FastMemberMapHelper.GetMembersBitArray(stackalloc uint[bitArrayLength]) : FastMemberMapHelper.GetMembersBitArray(bitArrayLength);
Copy link
Contributor

Choose a reason for hiding this comment

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

I was confused by the name bitArrayLength because it sounded like it was the number of bits, but it's actually the number of uints.

I suggest some renaming:

var (lengthInUInts, useStackAlloc) = FastMemberMapHelper.GetLengthInUInts(allMemberMaps.Count);
using var bitArray = useStackAlloc ? FastMemberMapHelper.GetMembersBitArray(stackalloc uint[lengthInUInts]) : FastMemberMapHelper.GetMembersBitArray(lengthInUInts);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done.


bsonReader.ReadStartDocument();
var elementTrie = _classMap.ElementTrie;
Expand Down Expand Up @@ -193,7 +197,8 @@ public TClass DeserializeClass(BsonDeserializationContext context)
DeserializeExtraElementValue(context, values, elementName, memberMap);
}
}
memberMapBitArray[memberMapIndex >> 5] |= 1U << (memberMapIndex & 31);

bitArray.SetMemberIndex(memberMapIndex);
}
else
{
Expand Down Expand Up @@ -221,7 +226,7 @@ public TClass DeserializeClass(BsonDeserializationContext context)
{
DeserializeExtraElementValue(context, values, elementName, extraElementsMemberMap);
}
memberMapBitArray[extraElementsMemberMapIndex >> 5] |= 1U << (extraElementsMemberMapIndex & 31);
bitArray.SetMemberIndex(extraElementsMemberMapIndex);
}
else if (_classMap.IgnoreExtraElements)
{
Expand All @@ -239,51 +244,38 @@ public TClass DeserializeClass(BsonDeserializationContext context)
bsonReader.ReadEndDocument();

// check any members left over that we didn't have elements for (in blocks of 32 elements at a time)
for (var bitArrayIndex = 0; bitArrayIndex < memberMapBitArray.Length; ++bitArrayIndex)
var bitArraySpan = bitArray.Span;
for (var bitArrayIndex = 0; bitArrayIndex < bitArraySpan.Length; bitArrayIndex++)
{
var memberMapIndex = bitArrayIndex << 5;
var memberMapBlock = ~memberMapBitArray[bitArrayIndex]; // notice that bits are flipped so 1's are now the missing elements
var memberMapBlock = ~bitArraySpan[bitArrayIndex]; // notice that bits are flipped so 1's are now the missing elements

// work through this memberMapBlock of 32 elements
while (true)
for (; memberMapBlock != 0 && memberMapIndex < allMemberMaps.Count; memberMapIndex++, memberMapBlock >>= 1)
{
// examine missing elements (memberMapBlock is shifted right as we work through the block)
for (; (memberMapBlock & 1) != 0; ++memberMapIndex, memberMapBlock >>= 1)
{
var memberMap = allMemberMaps[memberMapIndex];
if (memberMap.IsReadOnly)
{
continue;
}

if (memberMap.IsRequired)
{
var fieldOrProperty = (memberMap.MemberInfo is FieldInfo) ? "field" : "property";
var message = string.Format(
"Required element '{0}' for {1} '{2}' of class {3} is missing.",
memberMap.ElementName, fieldOrProperty, memberMap.MemberName, _classMap.ClassType.FullName);
throw new FormatException(message);
}
if ((memberMapBlock & 1) == 0)
continue;

if (document != null)
{
memberMap.ApplyDefaultValue(document);
}
else if (memberMap.IsDefaultValueSpecified && !memberMap.IsReadOnly)
{
values[memberMap.ElementName] = memberMap.DefaultValue;
}
var memberMap = allMemberMaps[memberMapIndex];
if (memberMap.IsReadOnly)
{
continue;
}

if (memberMapBlock == 0)
if (memberMap.IsRequired)
{
break;
var fieldOrProperty = (memberMap.MemberInfo is FieldInfo) ? "field" : "property";
throw new FormatException($"Required element '{memberMap.ElementName}' for {fieldOrProperty} '{memberMap.MemberName}' of class {_classMap.ClassType.FullName} is missing.");
}

// skip ahead to the next missing element
var leastSignificantBit = FastMemberMapHelper.GetLeastSignificantBit(memberMapBlock);
memberMapIndex += leastSignificantBit;
memberMapBlock >>= leastSignificantBit;
if (document != null)
{
memberMap.ApplyDefaultValue(document);
}
else if (memberMap.IsDefaultValueSpecified && !memberMap.IsReadOnly)
{
values[memberMap.ElementName] = memberMap.DefaultValue;
}
}
}

Expand Down Expand Up @@ -335,13 +327,11 @@ public bool GetDocumentId(
idGenerator = idMemberMap.IdGenerator;
return true;
}
else
{
id = null;
idNominalType = null;
idGenerator = null;
return false;
}

id = null;
idNominalType = null;
idGenerator = null;
return false;
}

/// <summary>
Expand Down Expand Up @@ -694,48 +684,63 @@ private bool ShouldSerializeDiscriminator(Type nominalType)

// nested classes
// helper class that implements member map bit array helper functions
private static class FastMemberMapHelper
internal static class FastMemberMapHelper
{
public static uint[] GetBitArray(int memberCount)
internal ref struct MembersBitArray()
{
var bitArrayOffset = memberCount & 31;
var bitArrayLength = memberCount >> 5;
if (bitArrayOffset == 0)
{
return new uint[bitArrayLength];
}
var bitArray = new uint[bitArrayLength + 1];
bitArray[bitArrayLength] = ~0U << bitArrayOffset; // set unused bits to 1
return bitArray;
}
private readonly ArrayPool<uint> _arrayPool;
private readonly Span<uint> _bitArray;
private readonly uint[] _rentedBuffer;
private bool _isDisposed = false;

// see http://graphics.stanford.edu/~seander/bithacks.html#ZerosOnRightBinSearch
// also returns 31 if no bits are set; caller must check this case
public static int GetLeastSignificantBit(uint bitBlock)
{
var leastSignificantBit = 1;
if ((bitBlock & 65535) == 0)
{
bitBlock >>= 16;
leastSignificantBit |= 16;
}
if ((bitBlock & 255) == 0)
public MembersBitArray(Span<uint> bitArray) : this()
{
bitBlock >>= 8;
leastSignificantBit |= 8;
_arrayPool = null;
_bitArray = bitArray;
_rentedBuffer = null;

_bitArray.Clear();
}
if ((bitBlock & 15) == 0)

public MembersBitArray(int spanLength, uint[] rentedBuffer, ArrayPool<uint> arrayPool) : this()
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing in BOTH the rentedBuffer and the arrayPool it came from how about just passing in the arrayPool?

public MembersBitArray(int lengthInUInts, ArrayPool<uint> arrayPool) : this()
{                                                                            
    _arrayPool = arrayPool;                                                  
    _rentedBuffer = _arrayPool.Rent(lengthInUInts);                          
    _bitArray = _rentedBuffer.AsSpan(0, lengthInUInts);                      
                                                                             
    _bitArray.Clear();                                                       
}

Also suggest renaming spanLength to lengthInUInts to standardize on a name that clearly indicates it is the length in uints and not the length in bits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, it's cleaner. Done.

{
bitBlock >>= 4;
leastSignificantBit |= 4;
_arrayPool = arrayPool;
_bitArray = rentedBuffer.AsSpan(0, spanLength);
_rentedBuffer = rentedBuffer;

_bitArray.Clear();
}
if ((bitBlock & 3) == 0)

public Span<uint> Span => _bitArray;
public ArrayPool<uint> ArrayPool => _arrayPool;

public void SetMemberIndex(int memberMapIndex) =>
_bitArray[memberMapIndex >> 5] |= 1U << (memberMapIndex & 31);

public void Dispose()
{
bitBlock >>= 2;
leastSignificantBit |= 2;
if (_isDisposed)
return;

if (_rentedBuffer != null)
{
_arrayPool.Return(_rentedBuffer);
}
_isDisposed = true;
}
return leastSignificantBit - (int)(bitBlock & 1);
}

public static (int BitArrayLength, bool UseStackAlloc) GetMembersBitArrayLength(int membersCount)
{
var length = (membersCount + 31) >> 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename length to lengthInUInts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

return (length, length <= 8); // Use stackalloc for up to 256 members
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Some suggested renamings for clarity:

public static (int LengthInUInts, bool UseStackAlloc) GetLengthInUInts(int membersCoun
{
    var lengthInUInts = (membersCount + 31) >> 5;
    return (lengthInUInts, lengthInUInts <= 8); // Use stackalloc for up to 256 members
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


public static MembersBitArray GetMembersBitArray(Span<uint> span) =>
new(span);

public static MembersBitArray GetMembersBitArray(int length) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename length to lengthInUInts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

new(length, ArrayPool<uint>.Shared.Rent(length), ArrayPool<uint>.Shared);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested refactoring to only pass the ArrayPool:

public static MembersBitArray GetMembersBitArray(int lengthInUInts) =>
    new(lengthInUInts, ArrayPool<uint>.Shared);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}
}
Loading