BenchmarkDotNet icon indicating copy to clipboard operation
BenchmarkDotNet copied to clipboard

Custom names for argument/parameter values

Open IdanZel opened this issue 3 years ago • 4 comments
trafficstars

Implementation of #1634

I wrote this code quite a while ago and only decided to open a PR now, so let me know if there's anything outdated.

IdanZel avatar Dec 13 '21 21:12 IdanZel

CLA assistant check
All CLA requirements met.

dnfadmin avatar Dec 13 '21 21:12 dnfadmin

I'm not quiet sure yet, but this seems a bit overcomplicated and changes to SmartParamBuilder.cs aren't really necessary for this to work. I had written something similar a shorter while ago (ParamWrapper<T>, here is an example), although it lacks the fluent-style WithXxx() syntax and the Benchmark method expects the wrapped argument or parameter (I simply hadn't thought of adding an conversion).

public class ParamWrapper<T> : IDisposable
{
    public T Value;
    public string? DisplayText;
    public override string? ToString()
        => DisplayText ?? Value?.ToString() ?? ParameterInstance.NullParameterTextRepresentation;
    public ParamWrapper(T value, string? displayText)
    {
        Value = value;
        DisplayText = displayText;
    }
    public void Dispose() => (Value as IDisposable)?.Dispose();
    // Newly added just now to make the wrapper transparent to the benchmark method.
    public static implicit operator T(ParamWrapper<T> @this) => @this.Value;
}

Except for the fluent API, this is really all that's needed, and as far as I can tell, BDN will already do all the necessary type casting.

Usage example
// Usage 
public IEnumerable<ParamWrapper<MemoryStream>> DataSource_Wrapped()
{
    yield return new ParamWrapper<MemoryStream>(new MemoryStream(50), "small stream");
    yield return new ParamWrapper<MemoryStream>(new MemoryStream(500), "big stream");
}

[ParamsSource(nameof(DataSource_Wrapped))]
public MemoryStream InputStream { get; set; } = null!;

[Benchmark]
[ArgumentsSource(nameof(DataSource_Wrapped))]
public void Method1(MemoryStream input)
{
    input.Seek(0, SeekOrigin.Begin);
    byte[] buffer = new byte[10];
    while (input.Read(buffer, 0, buffer.Length) == buffer.Length) { }
}

[Benchmark]
public void Method2()
{
    InputStream.Seek(0, SeekOrigin.Begin);
    byte[] buffer = new byte[10];
    while (InputStream.Read(buffer, 0, buffer.Length) == buffer.Length) { }
}

mawosoft avatar Dec 14 '21 01:12 mawosoft

I like the approach suggested by @mawosoft that doesn't modify SmartParamBuilder and his friends. A few comments:

  1. Since this wrapper could be applied to both parameters and arguments, I would avoid the Param part in the class name. E.g, we can rename it to NamedValue.
  2. I would also add an implicit cast from T to the wrapper class so that we could easily introduce NamedValue instances without a specific name. Thus, the new implementation could look as follows:
public class NamedValue<T> : IDisposable
{
    public T Value { get; }
    public string? DisplayText { get; }

    public override string? ToString()
        => DisplayText ?? Value?.ToString() ?? ParameterInstance.NullParameterTextRepresentation;

    public NamedValue(T value, string? displayText)
    {
        Value = value;
        DisplayText = displayText;
    }

    public void Dispose() => (Value as IDisposable)?.Dispose();

    public static implicit operator T(NamedValue<T> @this) => @this.Value;
    public static implicit operator NamedValue<T>(T value) => new(value, null);
}

P.S. I'm against exposing static extension methods that could be applied to any object (like static NamedParameter WithName(this object parameter, ...). It works nice in the suggested example, but it contaminates the list of available object methods in other places.

AndreyAkinshin avatar Dec 14 '21 10:12 AndreyAkinshin

@AndreyAkinshin In my own library, I was also pondering if the wrapper should implement Equals() and how far that should go (i.e. also IEquatable, etc?).

In allmost all cases, BDN uses the text representation to compare parameters and arguments, but ReturnValueValidator and ExecutionValidator compare values. However, both are quite old and don't support arguments, only parameters. Not sure how much they are used anymore.

mawosoft avatar Dec 14 '21 13:12 mawosoft

@IdanZel thank you for your contribution!

Since the suggestions have not been implemented for few months, I am closing it as it's stale. Feel free to address them and reopen the PR.

adamsitnik avatar Aug 24 '22 07:08 adamsitnik