Skip to content

Adds benchmark tests for ReadOnlySequence<T>.FirstSpan #410

Merged
billwert merged 5 commits intodotnet:masterfrom
maryamariyan:first-span
Apr 8, 2019
Merged

Adds benchmark tests for ReadOnlySequence<T>.FirstSpan #410
billwert merged 5 commits intodotnet:masterfrom
maryamariyan:first-span

Conversation

@maryamariyan
Copy link
Copy Markdown
Contributor

@maryamariyan maryamariyan commented Mar 28, 2019

Adds benchmark tests for ReadOnlySequence.FirstSpan covering all cases

Related to https://github.com/dotnet/corefx/issues/33029

cc: @adamsitnik @ahsonkhan

Comment thread src/benchmarks/micro/MicroBenchmarks.csproj
Comment thread src/benchmarks/micro/corefx/System.Buffers/ReadOnlySequenceTests.cs Outdated
Comment thread src/benchmarks/micro/corefx/System.Buffers/ReadOnlySequenceTests.cs Outdated
@billwert
Copy link
Copy Markdown
Contributor

I didn't leave a comment on each instance of the new underscores. They should all be fixed. Thanks!

Comment thread src/benchmarks/micro/MicroBenchmarks.csproj Outdated
Comment thread src/benchmarks/micro/corefx/System.Buffers/ReadOnlySequenceTests.cs Outdated
Comment thread src/benchmarks/micro/MicroBenchmarks.csproj
@jorive jorive requested a review from billwert April 4, 2019 22:29
Comment thread src/benchmarks/micro/corefx/System.Buffers/ReadOnlySequenceTests.cs
[Benchmark(OperationsPerInvoke = 16)]
public int FirstArray() => First(new ReadOnlySequence<T>(_array));

#if NETCOREAPP3_0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@billwert, @jorive, I am just curious, is this the expected way to single out API benchmarks that only exist on certain TFMs? What if I want to add benchmarks for types that only exist on netstandard2.0, or only exist on .NET Core 2.0+, etc.?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good question. I don't think we have a solid plan yet. (And this approach is fragile, of course.) @adamsitnik may have considered this - let's see what he says when he's back from vacation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ahsonkhan it depends. if it's a single method or just a few like here, you can use #if TFM. If it's an entire type, you should exclude it per certain TFM in the .csproj file

<ItemGroup Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework' Or '$(TargetFramework)' == 'netcoreapp2.0'">
<Compile Remove="corefx\System.IO.Compression\Brotli.cs" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure, I figured that would be the expected approach. That's what I ended up doing for the recent JsonWriter perf tests. Thanks for clarifying.

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.

5 participants