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.

Sign up for GitHub

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

Jump to bottom

Benchmarks for ReadOnlySequence.Slice. #589

Merged
merged 8 commits into from
Jul 18, 2019
Merged

Benchmarks for ReadOnlySequence.Slice. #589

merged 8 commits into from
Jul 18, 2019

Conversation

pgovind
Copy link
Contributor

@pgovind pgovind commented Jun 26, 2019

Contains hacks to get it working in VS

NOT to be merged yet. I'm just putting this up for dotnet/corefx#38431

Benchmarks for ReadOnlySequence.Slice.
3bd4e53 
Contains hacks to get it working in VS
@pgovind pgovind mentioned this pull request Jun 26, 2019
Merged
@ahsonkhan
Copy link
Member

ahsonkhan commented Jun 26, 2019 •
edited
Loading

Contains hacks to get it working in VS

cc @adamsitnik, @billwert - what can we do to make the experience within the perf repo better for testing latest?

ahsonkhan
ahsonkhan reviewed Jun 26, 2019
View reviewed changes
src/benchmarks/micro/corefx/System.Memory/ReadOnlySequence.SequenceReader.cs Outdated Show resolved Hide resolved
src/benchmarks/micro/corefx/System.Memory/ReadOnlySequence.SequenceReader.cs Outdated Show resolved Hide resolved
src/benchmarks/micro/corefx/System.Memory/ReadOnlySequence.cs Outdated Show resolved Hide resolved
src/benchmarks/micro/corefx/System.Memory/ReadOnlySequence.cs Outdated Show resolved Hide resolved
src/benchmarks/micro/corefx/System.Memory/ReadOnlySequence.cs Outdated
public ReadOnlySequence<byte> MS_RepeatSlice_StartPosition_And_EndPosition()
{
var localSequence = _multiSegmentSequence.Slice(_ms_start, _ms_end);
localSequence = localSequence.Slice(_ms_start, localSequence.End);
Copy link
Member

Choose a reason for hiding this comment

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

It's probably better to chose different start/end positions for slicing rather than reusing the same one, but it probably doesn't matter.

src/benchmarks/micro/corefx/System.Memory/ReadOnlySequence.cs Outdated

// Add multiple calls for Slice(startPosition, endPosition)
[Benchmark]
public ReadOnlySequence<byte> MS_RepeatSlice_StartPosition_And_EndPosition()
Copy link
Member

Choose a reason for hiding this comment

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

Can you share the result of this benchmark (along with some of the other missing ones)? I only saw a few mentioned in dotnet/corefx#38431 (comment).

@pgovind
Copy link
Contributor Author

pgovind commented Jun 26, 2019 •
edited
Loading

So, after addressing @ahsonkhan's comments, I see this when I run the benchmarks.

C:\Users\prgovi\Desktop\Work\performance\src\tools\ResultsComparer>dotnet run --base "C:\Users\prgovi\Desktop\Work\Benchmarks\Before" --diff "C:\Users\prgovi\Desktop\Work\Benchmarks\After" --threshold 2%
summary:
better: 5, geomean: 1.057
worse: 3, geomean: 1.152
total diff: 8

Slower diff/base Base Median (ns) Diff Median (ns) Modality
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.MS_StartPosition 1.24 8.44 10.47
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.StartPosition 1.17 4.97 5.79
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.MS_StartPosition_And_EndPositi 1.06 11.02 11.65
Faster base/diff Base Median (ns) Diff Median (ns) Modality
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.RepeatSlice_StartPosition_And_ 1.08 24.39 22.56
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.Start_And_EndPosition 1.07 6.55 6.10
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.StartPosition_And_EndPosition 1.07 5.99 5.61
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.RepeatSlice 1.03 66.89 64.63
MicroBenchmarks.corefx.ReadOnlySequenceBenchmarks.MS_RepeatSlice 1.03 81.18 79.00

@ahsonkhan

@adamsitnik
Copy link
Member

Contains hacks to get it working in VS

what can we do to make the experience within the perf repo better for testing latest?

I have fixed that in #594 Now to get it working in VS you should just install the latest Core SDK from https://github.com/dotnet/core-sdk#installers-and-binaries

@ahsonkhan
Copy link
Member

I have fixed that in #594

@pgovind - could you please verify this works in VS as you would expect now and update this PR so it's ready to be merged?

@adamsitnik
Copy link
Member

@pgovind please excuse me for such a delay in the reviewing process. I am now doing 2.2 vs 3.0 comparison and I really needed these benchmarks so I just pushed the fixes to your branch. PTAL at the changes that I've introduced.

@adamsitnik
Copy link
Member

Sample results:

dotnet run -c Release -f netcoreapp3.0 --filter SequenceReader
Method Mean Error StdDev Median Min Max Gen 0/1k Op Gen 1/1k Op Gen 2/1k Op Allocated Memory/Op
TryReadTo 198.7 ns 1.471 ns 1.376 ns 198.4 ns 197.0 ns 201.5 ns - - - - 
dotnet run -c Release -f netcoreapp3.0 --filter ReadOnlySequence
Method Segment Mean Error StdDev Median Min Max Gen 0/1k Op Gen 1/1k Op Gen 2/1k Op Allocated Memory/Op
Slice_StartPosition Single 6.171 ns 0.2860 ns 0.3294 ns 6.316 ns 5.673 ns 6.649 ns - - - -
Slice_Start Single 10.097 ns 0.0748 ns 0.0663 ns 10.128 ns 9.955 ns 10.177 ns - - - -
Slice_Start_And_Length Single 12.756 ns 0.0766 ns 0.0716 ns 12.747 ns 12.645 ns 12.860 ns - - - -
Slice_Start_And_EndPosition Single 6.793 ns 0.0736 ns 0.0688 ns 6.763 ns 6.714 ns 6.918 ns - - - -
Slice_StartPosition_And_Length Single 7.099 ns 0.0592 ns 0.0553 ns 7.092 ns 7.032 ns 7.211 ns - - - -
Slice_StartPosition_And_EndPosition Single 8.407 ns 0.0549 ns 0.0514 ns 8.410 ns 8.344 ns 8.526 ns - - - -
Slice_Repeat Single 70.908 ns 0.7022 ns 0.6568 ns 70.713 ns 70.046 ns 72.189 ns - - - -
Slice_Repeat_StartPosition_And_EndPosition Single 25.973 ns 0.2128 ns 0.1991 ns 25.914 ns 25.719 ns 26.398 ns - - - -
Slice_StartPosition Multiple 11.185 ns 0.0768 ns 0.0719 ns 11.167 ns 11.092 ns 11.345 ns - - - -
Slice_Start Multiple 10.068 ns 0.0827 ns 0.0774 ns 10.104 ns 9.915 ns 10.140 ns - - - -
Slice_Start_And_Length Multiple 18.785 ns 0.1607 ns 0.1503 ns 18.853 ns 18.528 ns 18.958 ns - - - -
Slice_Start_And_EndPosition Multiple 13.287 ns 0.1012 ns 0.0947 ns 13.268 ns 13.154 ns 13.481 ns - - - -
Slice_StartPosition_And_Length Multiple 13.363 ns 0.0951 ns 0.0889 ns 13.343 ns 13.260 ns 13.554 ns - - - -
Slice_StartPosition_And_EndPosition Multiple 14.448 ns 0.0898 ns 0.0840 ns 14.495 ns 14.314 ns 14.546 ns - - - -
Slice_Repeat Multiple 85.813 ns 1.6846 ns 1.5758 ns 85.161 ns 83.973 ns 88.796 ns - - - -
Slice_Repeat_StartPosition_And_EndPosition Multiple 59.594 ns 0.3184 ns 0.2823 ns 59.531 ns 59.248 ns 60.268 ns - - - -

@adamsitnik adamsitnik merged commit dba17a2 into dotnet:master Jul 18, 2019
This was referenced Jul 18, 2019
Merged
Merged
Merged
@ahsonkhan
Copy link
Member

I am now doing 2.2 vs 3.0 comparison and I really needed these benchmarks so I just pushed the fixes to your branch. PTAL at the changes that I've introduced.

Great. It would be great to verify that we didn't significantly regress ROSequence.Slice between 2.2 and 3.0.

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

@ahsonkhan ahsonkhan ahsonkhan left review comments

Assignees
No one assigned
Labels
*NO MERGE*
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pgovind @ahsonkhan @adamsitnik @jorive