CSHARP-5368: Eliminiate allocations in Bson.Decimal128 to decimal conversion#1515
CSHARP-5368: Eliminiate allocations in Bson.Decimal128 to decimal conversion#1515papafe merged 4 commits intomongodb:mainfrom
Conversation
|
Hi @obligaron thanks a lot for your PR. I'll take a look, but in the meantime could you rebase on the latest main? We did some changes to the CI and most tests will fail if you don't. |
f032aed to
3cd2b0a
Compare
|
I rebased the PR. Thanks @papafe for looking at the code. 🙂 |
| var yType = GetDecimal128Type(y); | ||
| var result = xType.CompareTo(yType); | ||
| #if NET6_0_OR_GREATER | ||
| var result = Comparer<Decimal128Type>.Default.Compare(xType, yType); |
There was a problem hiding this comment.
It seems that Comparer.Default is available since .NET Framework 2.0. Is there a specific reason why we're making a cutoff point at .NET 6.0 here?
There was a problem hiding this comment.
.NET Framework lacks some optimisations for enums (e.g. dotnet/coreclr#5503 or dotnet/coreclr#14125).
This results in boxing and lookup overhead for the default comparer on .NET Framework.
That's why I specially cased it to avoid this overhead.
Alternatives could be
- Change the .NET Framework implementation to
Comparer.Defaultand accept the boxing/overhead (this is slightly slower than the current implementation in main). - Change the .NET 6 implementation to the int-cast logic of the .NET Framework (this is almost identical, but less readable).
I'm open to all three (keep it or change to one of the alternatives). 🙂
papafe
left a comment
There was a problem hiding this comment.
LGTM!
Thanks a lot for this PR @obligaron
See CSHARP-5368 for details.
This PR removes allocations that happens for
Enum.CompareTo(object? enum), because this results in boxing of both enums.The alternative is
Comparer<T>.Default.Comparfor .NET 6.0 and manual int-casts for .NET 4.x.decimal.GetBits(decimal d)allocates anint[4]array on every call.The alternative is
decimal.GetBits(decimal d, Span<int> destination), which allows you to pass a stack allocated int-array. This method is not available in .NET 4.x.