Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Benchmarks cleanup + Readme #2394

Merged
merged 6 commits into from Jul 13, 2018
Merged

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Jul 13, 2018

@JeremyKuhne you have asked me yesterday how to run the benchmarks and then I realized that this repo lacks of README.md file for the benchmarks

  1. I added the readme
  2. I removed old xunit.performance stuff
  3. I ported last remaining benchmarks to BenchmarkDotNet
  4. I updated BDN to latest version to take advantage of latest features (including glob filtering from command line)

/cc @KrzysztofCwalina @ahsonkhan

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

Thanks @adamsitnik!

@JeremyKuhne JeremyKuhne merged commit 9aedb96 into dotnet:master Jul 13, 2018

(`..\..\dotnetcli\dotnet.exe run -c Release`)

3b. To run specific tests only, pass in the filter to the harness:
Copy link
Member

Choose a reason for hiding this comment

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

Nice!


1. Navigate to the benchmarks directory (corefxlab\tests\Benchmarks\)

2. Restore this project
Copy link
Member

Choose a reason for hiding this comment

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

Is this step necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I forgot that dotnet run does restore the project as well


namespace System.IO.Pipelines.Benchmarks
{
[SimpleJob(RunStrategy.Monitoring)] // the setting for benchmarks with not-steady state
Copy link
Member

Choose a reason for hiding this comment

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

What changed that allows us to remove this attribute?

Copy link
Member Author

Choose a reason for hiding this comment

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

I made BDN smarter ;) Now when the benchmark is very time consuming it will run it once per iteration, not 16 times as it used to.

{
public partial class PrimitiveParserPerfTests
{
[Benchmark]
Copy link
Member

Choose a reason for hiding this comment

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

Can we mark this benchmark as baseline?

Copy link
Member Author

Choose a reason for hiding this comment

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

there can be only 1 baseline benchmark per type so if you split PrimitiveParserPerfTests into multiple types then yes


[Benchmark]
[ArgumentsSource(nameof(Utf8ByteArrays))]
public void PrimitiveParserByteSpanToBool(Utf8ByteArrayArgument text) => Utf8Parser.TryParse(text.CreateSpan(), out bool value, out int bytesConsumed);
Copy link
Member

@ahsonkhan ahsonkhan Jul 16, 2018

Choose a reason for hiding this comment

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

Anyway to get rid of the overhead for creating the span? Does it have much affect on the end results of these benchmarks, given parsing primitives like bool/ints is quite fast?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ahsonkhan this is a very good question.

I believe that this particular parsing operation should include the cost of the Span creation. Why? Because even if we have some huge buffer of memory and we want to parse just a part of it using the Span API we need to create a Span. So we can't use it without creating a Span instance.

If we would like to amortize the cost of Span creation we could use the OperationsPerInvoke feature:

[Benchmark(OperationsPerInvoke = 10)]
[ArgumentsSource(nameof(Utf8ByteArrays))]
public void PrimitiveParserByteSpanToBool(Utf8ByteArrayArgument text)
{
     bool result = default;
     var span = text.CreateSpan();

     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 1
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 2
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 3
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 4
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 5
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 6
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 7
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 8
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 9 
     result = Utf8Parser.TryParse(span, out bool value, out int bytesConsumed); // 10

    return result;
}

"2147483647",
"21",
"2147483",
"-214"
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be worthwhile to add test cases like these, no?
"000000000000000000001235abcdfg"
"2abcdefghijklmnop"

@ahsonkhan
Copy link
Member

  1. I added the readme
  2. I removed old xunit.performance stuff
  3. I ported last remaining benchmarks to BenchmarkDotNet
  4. I updated BDN to latest version to take advantage of latest features (including glob filtering from command line)

Awesome! Thanks for taking the time to do this @adamsitnik.

FYI, @L-Dogg

@ahsonkhan
Copy link
Member

I realized that this repo lacks of README.md file for the benchmarks

We should update this section at the root of the repo to point to the README you added:
https://github.com/dotnet/corefxlab#measuring-performance

(`..\..\dotnetcli\dotnet.exe run -c Release -- --filter namespace*`)
(`..\..\dotnetcli\dotnet.exe run -c Release -- --filter *typeName*`)
(`..\..\dotnetcli\dotnet.exe run -c Release -- --filter *.methodName`)
(`..\..\dotnetcli\dotnet.exe run -c Release -- --filter namespace.typeName.methodName`)
Copy link
Member

Choose a reason for hiding this comment

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

nit: The steps are not getting formatted correctly when we view the README within the browser.

  • The numbered list is not aligned correctly.
  • The various filter commands all end up on the same line.

image

@adamsitnik adamsitnik deleted the benchmarksHowTo branch July 17, 2018 14:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants