Trying to evaluate two classes with a BenchmarkDotNet test

Question

I'm trying to figure out which one of these classes below is faster, so I decided to write a BenchmarkDotNet test.

public class RequestSignerBenchmark
{
    [Benchmark]
    public void First()
    {
        var signer = RequestSigner2.FromSecret("hidden");

        var timestamp = TimestampProvider.Default.CurrentTimestamp();
        signer.Sign(timestamp, "GET", "products", "");
    }

    [Benchmark]
    public void Second()
    {
        var signer = RequestSigner.FromSecret("hidden");

        var timestamp = TimestampProvider.Default.CurrentTimestamp();
        signer.Sign(timestamp, "GET", "products", "");
    }
}

// Program.cs
BenchmarkRunner.Run<RequestSignerBenchmark>();

I'm not sure that I'm running that the test I made is correct. Isn't the First() supposed to be faster judging by the code? If it's wrong, what would an appropriate test would look like?

| Method |     Mean |     Error |    StdDev |
|------- |---------:|----------:|----------:|
|  First | 3.234 us | 0.0646 us | 0.1769 us |
| Second | 2.659 us | 0.0516 us | 0.0614 us |

Classes to evaluate

public sealed class RequestSigner
{
    private readonly byte[] _base64Secret;

    public RequestSigner(byte[] base64Secret)
    {
        _base64Secret = base64Secret;
    }

    public static RequestSigner FromSecret(string secret)
    {
        return new RequestSigner(Convert.FromBase64String(secret));
    }

    public string Sign(string timestamp, string method, string requestPath, string body)
    {
        var orig = Encoding.UTF8.GetBytes(timestamp + method.ToUpperInvariant() + requestPath + body);
        using var hmac = new HMACSHA256(_base64Secret);
        return Convert.ToBase64String(hmac.ComputeHash(orig));
    }
}

public class RequestSigner2 : IDisposable
{
    private readonly HMACSHA256 _hmac;

    public RequestSigner2(byte[] base64Secret)
    {
        _hmac = new HMACSHA256(base64Secret);
    }

    public static RequestSigner2 FromSecret(string secret)
    {
        return new RequestSigner2(Convert.FromBase64String(secret));
    }

    public string Sign(string timestamp, string method, string requestPath, string body)
    {
        DoDisposeChecks();

        if (string.IsNullOrWhiteSpace(method) || string.IsNullOrWhiteSpace(requestPath))
        {
            return string.Empty;
        }

        var orig = Encoding.UTF8.GetBytes(timestamp + method.ToUpperInvariant() + requestPath + body);
        return Convert.ToBase64String(_hmac.ComputeHash(orig));
    }

    #region Disposable

    private bool _isDisposed;

    /// <summary>
    ///     Checks if this object has been disposed.
    /// </summary>
    /// <exception cref="ObjectDisposedException">Thrown if the object has been disposed.</exception>
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    protected void DoDisposeChecks()
    {
        if (_isDisposed)
        {
            throw new ObjectDisposedException(nameof(RequestSigner2));
        }
    }

    /// <inheritdoc />
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    /// <summary>
    ///     Disposes of managed and unmanaged resources.
    /// </summary>
    /// <param name="disposing">A value indicating whether or not to dispose of managed resources.</param>
    protected virtual void Dispose(bool disposing)
    {
        if (_isDisposed)
        {
            return;
        }

        if (disposing)
        {
            _hmac.Dispose();
        }

        _isDisposed = true;
    }

    #endregion
}

Answer 1

As I mentioned in the comments above, I believe your assertion that the RequestSigner2 implementation should be faster "may" be incorrect based on how your test is currently structured.

You have two additional if checks in your RequestSigner2 implementation that are having an impact on your performance (and we're talking about a nanosecond here...) if you do not reuse your HMACSHA256 instance correctly, and currently you are not.

By removing both the checks, and wrapping your initialization of RequestSigner2 in a using statement,

[Benchmark]
public void First()
{
    using var signer = RequestSigner2.FromSecret("aGlkZGVu");

    var timestamp = DateTime.UtcNow.ToString();
    signer.Sign(timestamp, "GET", "products", "");
}

I was able to achieve the following.

Note that your "optimized" version actually allocates 8 more bytes.

I am guessing that your assumption was that reusing the HMACSHA256 instance would be cheaper than initializing it each call. But interestingly enough, this is more expensive in your test.

I believe that you intend to reuse the signer in your actual application code, but this isn't mirrored in your benchmark code.

You should consider initializing the RequestSigner2 once in a GlobalSetup step, then dispose it in a GlobalCleanup step.

[MemoryDiagnoser]
public class RequestSignerBenchmark
{
    private RequestSigner2 _requestSigner2;

    [GlobalSetup]
    public void Setup()
    {
        _requestSigner2 = RequestSigner2.FromSecret("aGlkZGVu");
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        _requestSigner2.Dispose();
    }

    [Benchmark]
    public void First()
    { 
        var timestamp = DateTime.UtcNow.ToString();
        _requestSigner2.Sign(timestamp, "GET", "products", "");
    }

    [Benchmark]
    public void Second()
    {
        var signer = RequestSigner.FromSecret("aGlkZGVu");

        var timestamp = DateTime.UtcNow.ToString();
        signer.Sign(timestamp, "GET", "products", "");
    }
}

(Note that I've had to change a few parts of the benchmark implementation, due to not having your Timezone implementation).

Now we get the performance improvements you were expecting, even with both if checks added back in!