Skip to content

GH-44237: [C#] Use stack allocated buffer when serializing decimal values#44238

Merged
CurtHagenlocher merged 1 commit intoapache:mainfrom
georgevanburgh:decimal-getbits-overload
Sep 26, 2024
Merged

GH-44237: [C#] Use stack allocated buffer when serializing decimal values#44238
CurtHagenlocher merged 1 commit intoapache:mainfrom
georgevanburgh:decimal-getbits-overload

Conversation

@georgevanburgh
Copy link
Copy Markdown
Contributor

@georgevanburgh georgevanburgh commented Sep 26, 2024

Span overrides for decimal.GetBits were added in .NET 5, and allow us to avoid a heap allocation for each decimal value serialized.

Running a quick benchmark shows the expected reduction in allocations

using BenchmarkDotNet.Attributes;

namespace Apache.Arrow.Benchmarks;

[MemoryDiagnoser]
public class DecimalUtilityBenchmark
{
    public decimal Value => 1.00000000m;

    private byte[] _buffer = new byte[16];

    [Benchmark(Baseline = true)]
    public void Baseline() => DecimalUtilityMain.GetBytes(Value, 34, 10, 16, _buffer);

    [Benchmark]
    public void Candidate() => DecimalUtility.GetBytes(Value, 34, 10, 16, _buffer);
}
BenchmarkDotNet v0.14.0, Windows 10 (10.0.19045.4894/22H2/2022Update)
13th Gen Intel Core i7-13800H, 1 CPU, 20 logical and 14 physical cores
.NET SDK 8.0.304
  [Host]     : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
Method Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
Baseline 162.7 ns 3.25 ns 4.23 ns 1.00 0.04 0.0088 112 B 1.00
Candidate 152.8 ns 1.38 ns 1.22 ns 0.94 0.02 0.0057 72 B 0.64

Happy to check in some benchmarks for DecimalUtility if they might be useful in future, but thought I'd leave them out for now.

If merged, will close #44237.

Span overrides for decimal.GetBits were added in .NET 5, and allow us to
avoid a heap allocation for each decimal value serialized.
@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #44237 has been automatically assigned in GitHub to PR creator.

Copy link
Copy Markdown
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement!

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 26, 2024
@CurtHagenlocher CurtHagenlocher merged commit dcf0d8c into apache:main Sep 26, 2024
@CurtHagenlocher CurtHagenlocher removed the awaiting committer review Awaiting committer review label Sep 26, 2024
@georgevanburgh georgevanburgh deleted the decimal-getbits-overload branch September 26, 2024 12:36
@conbench-apache-arrow
Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit dcf0d8c.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 10 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C#] Use new decimal.GetBits overloads to reduce allocations when serializing decimal values

2 participants