Repository navigation
Allow for benchmark categories composition using inheritance #2306
Description
I am writing some benchmarks over generic classes, and to do so I created base generic benchmark, something along these lines
[BenchmarkCategory("Number")]
public class MyBenchmark<T> where T : unmanaged, INumberBase<T>
{
public T V1;
public T V2;
[BenchmarkCategory("Add")]
public T Add()
{
return V1 + V2;
}
[BenchmarkCategory("Multiply")]
public T Multiply()
{
return V1 * V2;
}
[GlobalSetup]
public virtual void Setup()
{
V1 = T.One + T.One;
V2 = T.One + T.One + T.One;
}
}Actual benchmarks are specialized over the generic type
[BenchmarkCategory("Float")]
public class MyFloatBenchmark : MyBenchmark<float>
{
[BenchmarkCategory("Sqrt")]
public float Sqrt()
{
return float.Sqrt(V1);
}
}The problem is that the category defined on the derived class is not assigned to the methods declared on the base class, because BenchmarkConverter only looks at the method declaring type. Moreover, the attribute on the derived class replaces the one defined on the base class, because the current resolution strategy ignores inherited attributes.
Is there a reason why method.ReflectedType is not used in this case and inherited attributes are explicitly ignored (both at the class and method level)?
So far, the only option is to replicate the categories on both the derived class and overridden method.
Would it be possible to add an extension point to the BenchmarkConverter (e.g., an ICategoryResolver or a CategoryResolver delegate) to plug-in a different categories discovery logic?
Activity
Enabled category inheritance, fix #2306
Introduce CategoryDiscoverer, fix #2306
AndreyAkinshin commentedon May 14, 2023
@BladeWise Thanks for the feedback. In #2307, I have implemented the following:
[CategoryDiscoverer(false)].IntroCategoryDiscovererandCategoriesTestsin Introduce CategoryDiscoverer, fix #2306 #2307 for usage examples)Does it solve your problem? Do you have any comments on the implementation?
BladeWise commentedon May 14, 2023
The PR solves the above issue perfectly, and the implementation is flexible enough to account for custom discovery strategies.
I am not sure if it would be worth to add an explicit configuration extension method to revert to the old behavior (or eventually opt-in to the new one), something like
The
CategoryDiscovererAttributeserves the same purpose, but an explicit extension method can be sometimes easier to discover.Thanks a lot for the quick response and support.
AndreyAkinshin commentedon May 14, 2023
Well, with the current state of the PR, it's possible to do
I believe there is no need for an additional shortcut method.
Great, glad to hear that. Then I merge the PR.
You're welcome!
Introduce CategoryDiscoverer, fix #2306
Enabled category inheritance, fix dotnet#2306