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 specify benchmark description in outputs #2386

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

Nepp3r
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

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 |

src/BenchmarkDotNet/Running/BenchmarkConverter.cs Outdated Show resolved Hide resolved
src/BenchmarkDotNet/Running/BenchmarkConverter.cs Outdated Show resolved Hide resolved
src/BenchmarkDotNet/Running/BenchmarkConverter.cs Outdated Show resolved Hide resolved
src/BenchmarkDotNet/Running/BenchmarkConverter.cs Outdated Show resolved Hide resolved
@Nepp3r
Copy link
Author

Nepp3r commented Aug 1, 2023

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?

@Nepp3r
Copy link
Author

Nepp3r commented Aug 1, 2023

@microsoft-github-policy-service agree

@timcassell
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?

@Nepp3r
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?

@timcassell
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.

@Nepp3r
Copy link
Author

Nepp3r commented Aug 8, 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?

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;
		}
	}

изображение

@timcassell
Copy link
Collaborator

timcassell commented Aug 8, 2023

@Nepp3r Try this

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

@timcassell
Copy link
Collaborator

timcassell commented Aug 16, 2023

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to configure benchmark class name in outputs
4 participants