Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Ability to specify benchmark description in outputs #2386

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

Open
Nepp3r wants to merge 21 commits into dotnet:master
base: master
Choose a base branch
Loading
from Nepp3r:classnameoutput

Conversation

Copy link

@Nepp3r Nepp3r commented Jul 31, 2023

Hello,
I think I've done this recomendations:
#1651 (comment)

If you have some other recomendations I am open to them.

Closes #1447
Supersedes #1651

cdonke and others added 13 commits July 27, 2023 15:32
...ptive description and removing only methods attribute from BenchmarkDescriptionAttribute
...hmarkConverterTests and remaking them in a correct way
Copy link
Collaborator

@timcassell timcassell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the approval tests, please compare 2 types, 1 with BenchmarkDescription and 1 without. The table should look something like this:

 Type | Method | Job | Mean | Error | StdDev | Rank | LogicalGroup | Baseline |
----------------------------------- |------- |----- |---------:|--------:|--------:|-----:|------------- |--------- |
JobBaseline_NoRenameJob_MethodsJobs | Base | Job1 | 102.0 ns | 6.09 ns | 1.58 ns | 1 | * | No |
JobBaseline_NoRenameJob_MethodsJobs | Foo | Job1 | 202.0 ns | 6.09 ns | 1.58 ns | 2 | * | No |
JobBaseline_NoRenameJob_MethodsJobs | Bar | Job1 | 302.0 ns | 6.09 ns | 1.58 ns | 3 | * | No |
 MyRenamedTestCase | Base | Job1 | 102.0 ns | 6.09 ns | 1.58 ns | 4 | * | No |
 MyRenamedTestCase | Foo | Job1 | 202.0 ns | 6.09 ns | 1.58 ns | 5 | * | No |
 MyRenamedTestCase | Bar | Job1 | 302.0 ns | 6.09 ns | 1.58 ns | 6 | * | No |

Copy link
Author

Nepp3r commented Aug 1, 2023
edited
Loading

For the approval tests, please compare 2 types, 1 with BenchmarkDescription and 1 without. The table should look something like this:

 Type | Method | Job | Mean | Error | StdDev | Rank | LogicalGroup | Baseline |
----------------------------------- |------- |----- |---------:|--------:|--------:|-----:|------------- |--------- |
JobBaseline_NoRenameJob_MethodsJobs | Base | Job1 | 102.0 ns | 6.09 ns | 1.58 ns | 1 | * | No |
JobBaseline_NoRenameJob_MethodsJobs | Foo | Job1 | 202.0 ns | 6.09 ns | 1.58 ns | 2 | * | No |
JobBaseline_NoRenameJob_MethodsJobs | Bar | Job1 | 302.0 ns | 6.09 ns | 1.58 ns | 3 | * | No |
 MyRenamedTestCase | Base | Job1 | 102.0 ns | 6.09 ns | 1.58 ns | 4 | * | No |
 MyRenamedTestCase | Foo | Job1 | 202.0 ns | 6.09 ns | 1.58 ns | 5 | * | No |
 MyRenamedTestCase | Bar | Job1 | 302.0 ns | 6.09 ns | 1.58 ns | 6 | * | No |

If I understand clearly, I need to add Type column to the table if class has BenchmarkDescription attribute with description, right? How can I change result table?

Copy link
Author

Nepp3r commented Aug 1, 2023

@microsoft-github-policy-service agree

Copy link
Collaborator

If I understand clearly, I need to add Type column to the table if class has BenchmarkDescription attribute with description, right? How can I change result table?

It will be added automatically if more than 1 type is benchmarked and they are joined together. @AndreyAkinshin Can you help with how to setup the approval tests for that?

Copy link
Author

Nepp3r commented Aug 6, 2023

It will be added automatically if more than 1 type

I tried to change MockReport.CreateSummary, so method could get an array of types, but it would require too many changes, so I refused from it. How exactly do I need to send more than one type to the summary?

Copy link
Collaborator

It will be added automatically if more than 1 type

I tried to change MockReport.CreateSummary, so method could get an array of types, but it would require too many changes, so I refused from it. How exactly do I need to send more than one type to the summary?

I think such changes are necessary. Please go ahead and add that functionality.

Copy link
Author

Nepp3r commented Aug 8, 2023
edited
Loading

It will be added automatically if more than 1 type

I tried to change MockReport.CreateSummary, so method could get an array of types, but it would require too many changes, so I refused from it. How exactly do I need to send more than one type to the summary?

I think such changes are necessary. Please go ahead and add that functionality.

Ok, but I tried to create new benchmark, like with instrument. I created 2 classes, both with benchmarked methods, but results were divided into 2 files, like it is 2 separate runs for benchmarking. Also I didn't find any such verified files in the tests/BenchmarkDotNet.Tests/Exporters/VerifiedFiles. Could you please give me the example of the test, which gave you that one result higher or explain, how can I put results of two methods from different class to one result table?

About the benchmarked classes, sending you the code:

internal class Program
	{
		static void Main(string[] args)
		{
			var results = BenchmarkRunner.Run<Demo>();
			var results2 = BenchmarkRunner.Run<Demo2>();
		}
	}
	public class Demo
	{
		[Benchmark]
		public string GetFullStringNormally()
		{
			string output = "";
			for (int i = 0; i < 100; i++)
				output+=i;
			return output;
		}
	}
	public class Demo2
	{
		[Benchmark]
		public string GetFullStringNormally2() {
			string output = "";
			for (int i = 0; i < 100; i++)
				output += i;
			return output;
		}
	}

изображение

Copy link
Collaborator

timcassell commented Aug 8, 2023
edited
Loading

@Nepp3r Try this

BenchmarkSwitcher.FromTypes(new[] { typeof(Demo), typeof(Demo2) }).RunAllJoined(args: args);

Copy link
Collaborator

timcassell commented Aug 16, 2023
edited
Loading

I actually think we should just use System.ComponentModel.DescriptionAttribute instead of creating a new BenchmarkDescriptionAttribute. We also could use it for [Params] fields/properties (doesn't need to be done in this PR).

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

Reviewers

@timcassell timcassell timcassell requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ability to configure benchmark class name in outputs

AltStyle によって変換されたページ (->オリジナル) /