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

Ability to configure benchmark class name in outputs #1447

Open
roji opened this issue May 2, 2020 · 6 comments · May be fixed by #1651 or #2386
Open

Ability to configure benchmark class name in outputs #1447

roji opened this issue May 2, 2020 · 6 comments · May be fixed by #1651 or #2386

Comments

@roji
Copy link
Member

roji commented May 2, 2020

An individual benchmark method's name can be overridden for output purposes via the Description property on BenchmarkAttribute (see #1243 for a related issue on this). However, there doesn't seem to be a way to do the same for the entire class.

My use case: we have a superclass of benchmarks being extended for different databases, so an abstract OpenConnectionBenchmarks class gets extended by SqliteOpenConnectionBenchmarks. However, in the outputs (CSV going into a database) it's desirable to have OpenConnectionBenchmarks, as the database is tracked elsewhere.

@adamsitnik
Copy link
Member

Hello @roji ;)

I want to make sure that I understand you correctly. So you have sth like the code below:

class OpenConnectionBenchmark
{
    [Benchmark] void Benchmark() {}
}
class SqliteOpenConnectionBenchmarks : OpenConnectionBenchmark { }

And you don't like the fact that the output results contain the full type name: SqliteOpenConnectionBenchmarks.Benchmark and you would like the SqliteOpenConnectionBenchmarks.csv file to contain results that use id OpenConnectionBenchmarks.Benchmark?

@roji
Copy link
Member Author

roji commented May 4, 2020

Yep, that's pretty much it. I worked around it by renaming the classes, but that's not ideal (now there are several classes with the same name, making it difficult to navigate in the IDE). So not urgent or anything, but it seems like there should be a way to determine output names both at the class and method level (ideally something consistent between the two as well).

@roji
Copy link
Member Author

roji commented May 6, 2020

Another scenario: have a Benchmarks postfix on class names (e.g. ConnectBenchmarks), but remove the postfix in outputs.

@adamsitnik adamsitnik added this to the v0.12.x milestone Oct 21, 2020
@adamsitnik
Copy link
Member

The best thing I can do as of today is to mark it as up-for-grabs and hope that someone is going to pick it up.

The contributor who is willing to work on this should:

  • Choose a good name and introduce a new class-only attribute that allows for changing the exported type name. It should be added here
  • add a new test to our exporter's tests that ensure that using the attribute has the desired effect on exported data. These tests can be used as an example.
  • extend BenchmarkConverter and detect when given type has this attribute defined and initialize a new instance of descriptor with the provided custom name
  • make sure that all places in the source code are using this name, not just benchmark.Descriptor.Type.Name

@JobaDiniz
Copy link
Contributor

I see that the PR #1651 has been opened for 2.5 years. If there's something wrong with it, can I grab this issue and make another one? What's the hold on merging that PR?

@timcassell
Copy link
Collaborator

timcassell commented Jul 21, 2023

@JobaDiniz I don't mind if you want to send a separate PR if @cdonke is unavailable to update his. Just take note of the comments left in #1651.

@Nepp3r Nepp3r linked a pull request Jul 31, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment