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

Baseline and Params(Source) #881

Open
DragonSA opened this issue Sep 22, 2018 · 10 comments
Open

Baseline and Params(Source) #881

DragonSA opened this issue Sep 22, 2018 · 10 comments

Comments

@DragonSA
Copy link

Is there a way to have a property set by [Params] or [ParamSource] where one of the parameters is the baseline?

@adamsitnik
Copy link
Member

Hello @DragonSA

No, there is no way to do that as of today. However, we are open to ideas and suggestions (how should we extend our API to make it possible?)

@adamsitnik adamsitnik self-assigned this Sep 22, 2018
@DragonSA
Copy link
Author

DragonSA commented Oct 1, 2018

Regarding the API, I would suggest something like [ParamSource(BaseIndex=2)] to make the 3rd item returns the baseline.

I tried to implement this manually (using BenchmarkCase and related classes), but I was not successful.

@adamsitnik
Copy link
Member

adamsitnik commented Jun 30, 2023

Some thoughts:

  • We could introduce a new type for wrapping the values like BenchmarkArgument<T>(T value, bool isBaseline = false) where we could specify whether the argument is a baseline or not, but it would require one more type, extra work for those who use Args/Params (not ArgumentSource/ParamSource) and the need of knowing that the type exists
  • We could extend @DragonSA idea and move the BaselineIndex to [BenchmarkAtrribute] itself, which would make it easier to discover, but on the other hand could be problematic for users who use more than one Params, which produces a cartesian product of all combinations and then it's not so easy to tell the index. But perhaps it would not be an issue?

@YegorStepanov thoughts?

@timcassell
Copy link
Collaborator

timcassell commented Jun 30, 2023

We could introduce a new type for wrapping the values like BenchmarkArgument(T value, bool isBaseline = false) where we could specify whether the argument is a baseline or not, but it would require one more type, extra work for those who use Args/Params (not ArgumentSource/ParamSource) and the need of knowing that the type exists

I was thinking of this also. I would do something similar to NUnit's TestCaseSource and TestCaseData. ArgumentsSource and ParamsSource treat IEnumerable<BenchmarkData> specially (as opposed to IEnumerable<object>).

public BenchmarkData(params object[] data)
public IEnumerable<BenchmarkData> GetParams()
{
    yield return new BenchmarkData(0, null) { IsBaseline = true };
    yield return new BenchmarkData(42, "Hello World");
}

@bbrk24
Copy link

bbrk24 commented Jul 1, 2023

  • We could extend @DragonSA idea and move the BaselineIndex to [BenchmarkAtrribute] itself, which would make it easier to discover, but on the other hand could be problematic for users who use more than one Params, which produces a cartesian product of all combinations and then it's not so easy to tell the index. But perhaps it would not be an issue?

Honestly, in my case, it would be beneficial to make one parameter the benchmark across all the others, something like

[Params(
  new(0) { IsBaseline = true },
  new(1)
)]
public int X { get; set; }

[Params(2, 3)]
public int Y { get; set; }

Where it uses X = 0, Y = 2 for the baseline to compare X = 1, Y = 2, and uses X = 0, Y = 3 for the baseline to compare X = 1, Y = 3.

@timcassell
Copy link
Collaborator

timcassell commented Jul 1, 2023

@bbrk24 Multiple params are combinatorial, so how would the baseline work in that case?

X = 0, Y = 2, Z = 5 (baseline)
X = 0, Y = 3, Z = 5 (baseline)
X = 0, Y = 2, Z = 6 (baseline)
X = 0, Y = 3, Z = 6 (baseline)
// Which baseline are we comparing to?
X = 1, Y = 2, Z = 5
X = 1, Y = 3, Z = 5
X = 1, Y = 2, Z = 6
X = 1, Y = 3, Z = 6

[Edit] Looking at it visually, I guess they are easy to map to their respective baselines by matching the non-baseline params.

What if you put a baseline on each param?

[Edit] I think that would work by still mapping the non-baseline values, and the baseline benchmarks would be only a combination of both baseline values.

[Params(
  new(0) { IsBaseline = true },
  new(1)
)]
public int X { get; set; }

[Params(
  new(2) { IsBaseline = true },
  new(3)
)]
public int Y { get; set; }

[Params(5, 6)]
public int Y { get; set; }
X = 0, Y = 2, Z = 5 (baseline)
X = 0, Y = 3, Z = 5
X = 1, Y = 2, Z = 5
X = 1, Y = 3, Z = 5
X = 0, Y = 2, Z = 6 (baseline)
X = 0, Y = 3, Z = 6
X = 1, Y = 2, Z = 6
X = 1, Y = 3, Z = 6

So, depending on where you put the baseline, you could have multiple baselines, or a single baseline, and the non-baseline benchmarks can be mapped to their respective baseline benchmarks.

Of course, ignoring that that syntax is illegal in C# (only allows constant values in attributes), and that would have to be in ParamsSource.

@YegorStepanov
Copy link
Contributor

So, depending on where you put the baseline, you could have multiple baselines, or a single baseline, and the non-baseline benchmarks can be mapped to their respective baseline benchmarks.

I like this idea and the output of the table.

I suggest adding [BaselineIndex(0)] attribute.

[BaselineIndex(0)]
[Params(true, false)]
public bool A;

[BaselineIndex(1)]
[Params(1, 2)]
public bool B;

[Params(3, 4)]
public bool C; 

// A=true B=2 is a baseline

BenchmarkData

yield return new BenchmarkData(0, null) { IsBaseline = true };

Do we really need a public magic type? Unlike an attribute, it can dynamically select the baseline index, but is that usefull?

@timcassell
Copy link
Collaborator

timcassell commented Jul 1, 2023

BenchmarkData

yield return new BenchmarkData(0, null) { IsBaseline = true };

Do we really need a public magic type? Unlike an attribute, it can dynamically select the baseline index, but is that usefull?

That is necessary for ArgumentsSource. I guess ParamsSource doesn't have to be the same since it can only ever be 1 element, so we could have BenchmarkArguments(params object[] arguments) and BenchmarkParam<T>(T param, bool isBaseline).

[Edit] I think I misunderstood the question. Yes, I think being able to dynamically select a baseline would be useful. At worst, it won't hurt. Also, I think it makes it clearer when using ParamsSource, since the data and the index wouldn't be in different places in the code.

@YegorStepanov
Copy link
Contributor

BenchmarkParam is not convenient for simple benchmarks.
The user have to refactor Params to the ParamsSource and find the name of the magic class in the documentation.

In that case, we can add both options :)

@timcassell
Copy link
Collaborator

Could knock out 2 birds with 1 stone using the BenchmarkParam/BenchmarkArgument types (#1634).

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

No branches or pull requests

5 participants