Skip to content
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.

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

Avoid heap allocations in ValueRipemd160 #125

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

wipiano
Copy link
Contributor

@wipiano wipiano commented Jun 2, 2024

Problems

The Ripemd160 class was replaced by ValueRipemd160 in PR #121. However, this change presents a few issues:

  • ValueRipemd160 is defined as a struct, yet it contains array fields allocated on the heap.
  • Being a mutable struct, instances of ValueRipemd160 will have invalid states when copied, which constitutes a breaking change.

Solutions

To address these issues, the following solutions are proposed:

  • Change the definition of ValueRipemd160 from struct to ref struct. By imposing the constraint that this struct can only exist on the stack, it reduces the possibility of common mistakes such as unintended defensive copying, and enables the use of stack space as a buffer.
  • Introduce a static ComputeHashDigest(Span<byte>) method for safer and simpler computation of RIPEMD-160 hash. Since handling mutable structs and stack-based buffers is prone to errors, this method allows for simple use cases without directly manipulating instances of the ValueRipemd160 struct.
  • Replace internal buffers with Span<T> instead of arrays, allowing stack allocation. By passing buffers allocated on the stack using stackalloc as Span<T> to the constructor, heap allocation can be completely avoided.

Benchmark


BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3672/23H2/2023Update/SunValley3)
AMD Ryzen 9 7940HS w/ Radeon 780M Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 8.0.300
  [Host]   : .NET 8.0.5 (8.0.524.21615), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  ShortRun : .NET 8.0.5 (8.0.524.21615), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Job=ShortRun  IterationCount=3  LaunchCount=1  
WarmupCount=3  

Method InputLength Mean Error StdDev Ratio Gen0 Allocated Alloc Ratio
Ripemd160 10 331.3 ns 11.60 ns 0.64 ns 1.00 0.0286 240 B 1.00
ValueRipemd160 10 298.0 ns 15.36 ns 0.84 ns 0.90 0.0200 168 B 0.70
ValueRipemd160_Optimized 10 289.9 ns 22.06 ns 1.21 ns 0.87 0.0057 48 B 0.20
Ripemd160 100 605.8 ns 23.67 ns 1.30 ns 1.00 0.0286 240 B 1.00
ValueRipemd160 100 599.9 ns 14.31 ns 0.78 ns 0.99 0.0200 168 B 0.70
ValueRipemd160_Optimized 100 584.5 ns 18.50 ns 1.01 ns 0.96 0.0057 48 B 0.20

Benchmark code

public class Bench
{
    [Params(10, 100)]
    public int InputLength;

    private byte[] data;

    [GlobalSetup]
    public void GlobalSetup()
    {
        data = new byte[InputLength];
        Random.Shared.NextBytes(data);
    }
    
    [Benchmark(Baseline = true)]
    public byte[] Ripemd160()
    {
        Ripemd160 ripe = new();
        ripe.Add(data, 0, data.Length);
        return ripe.HashDigest();
    }
    
    [Benchmark()]
    public byte[] ValueRipemd160()
    {
        ValueRipemd160 ripe = new();
        ripe.Add(data, 0, data.Length);
        return ripe.HashDigest();
    }
    
    [Benchmark()]
    public byte[] ValueRipemd160_Optimized()
    {
        return Ripemd160Benchmark.ValueRipemd160_Optimized.ComputeHashDigest(data);
    }
}

@wipiano wipiano force-pushed the ripemd160-ref-struct branch from c73b927 to e49cf6d Compare June 2, 2024 12:45
@wipiano wipiano changed the title avoid heap allocation in ValueRipemd160 Avoid heap allocations in ValueRipemd160 Jun 2, 2024
@shannonklaus shannonklaus merged commit 7c749ec into aerospike:stage Jun 4, 2024
2 checks passed
@shannonklaus
Copy link
Collaborator

Thank you for your pull request. It will be included in my next release, which should be in mid June

@wipiano wipiano deleted the ripemd160-ref-struct branch June 5, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants