Add BinaryWriter perf tests#1639
Conversation
|
/cc @adamsitnik |
adamsitnik
left a comment
adamsitnik
left a comment
There was a problem hiding this comment.
LGTM, thank you @GrabYourPitchforks !
| { | ||
| /// <summary> | ||
| /// A <see cref="Stream"/> that acts as a null sink for data. Overrides members that | ||
| /// <see cref="Stream.Null"/> does not. Used for benchmarking wrappers around Stream |
There was a problem hiding this comment.
thank you for adding a clear explanation why Stream.Null is not enough 👍
|
|
||
| public override void WriteByte(byte value) { } | ||
|
|
||
| #if NETCOREAPP2_1_OR_GREATER // these virtual methods only exist in .NET Core 2.1+ |
There was a problem hiding this comment.
this is something new to me. Are you sure that it works outside of dotnet/runtime? cc @ViktorHofer
There was a problem hiding this comment.
@terrajobst Aren't these defines part of the latest compilers? In theory they should work in any project type in any repo, even non-MSFT code.
| private char[] _inputAsChars; | ||
| private BinaryWriter _bw; | ||
|
|
||
| [Params(4, 16, 512, 10_000, 100_000, 500_000, 2_000_000)] |
There was a problem hiding this comment.
can we reduce the number of test cases to the number of different code paths? https://github.com/dotnet/performance/blob/master/docs/microbenchmark-design-guidelines.md#Code-Paths
There was a problem hiding this comment.
Sure, there should be 3 different code paths. How about 32 chars, 8k chars, and 2M chars?
There was a problem hiding this comment.
How about 32 chars, 8k chars, and 2M chars?
👍
|
BTW I'm not quite sure how to debug the CI failures. Looks like a .nupkg dependency is missing in one of the legs? |
|
CI is a real mess right now for a bunch of reasons, not the least of which today is that there's been Nuget outages. Don't worry about it. |
3 participants
Adds benchmarks for various
BinaryWriter.WriteAPIs.