Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Span: BinarySearch add Array baseline perf tests and remove TODO comments #25989

Merged
merged 4 commits into from
Jan 2, 2018
Merged

Span: BinarySearch add Array baseline perf tests and remove TODO comments #25989

merged 4 commits into from
Jan 2, 2018

Conversation

nietras
Copy link

@nietras nietras commented Dec 19, 2017

Some leftover work from #25777

Added BinarySearch perf tests using Array.BinarySearch for comparison as suggested by @ahsonkhan CC: @KrzysztofCwalina

From this it can be seen there are significant performance improvements when compare is fast i.e. for int, this is due to the improvements in the actual binary search code for Span.

Test Name Metric Iterations AVERAGE STDEV.S MIN MAX
ArrayBinarySearch_Int_FirstIndex(size: 1) Duration 100 1.410 0.025 1.310 1.478
ArrayBinarySearch_Int_FirstIndex(size: 10) Duration 100 1.780 0.042 1.681 1.889
ArrayBinarySearch_Int_FirstIndex(size: 100) Duration 100 2.083 0.044 2.014 2.177
ArrayBinarySearch_Int_FirstIndex(size: 1000) Duration 100 2.627 0.244 2.424 4.606
ArrayBinarySearch_Int_LastIndex(size: 1) Duration 100 1.361 0.039 1.308 1.491
ArrayBinarySearch_Int_LastIndex(size: 10) Duration 100 1.983 0.051 1.901 2.115
ArrayBinarySearch_Int_LastIndex(size: 100) Duration 100 2.730 0.061 2.586 2.857
ArrayBinarySearch_Int_LastIndex(size: 1000) Duration 100 3.129 0.062 2.955 3.246
ArrayBinarySearch_Int_MiddleIndex(size: 1) Duration 100 1.372 0.042 1.320 1.530
ArrayBinarySearch_Int_MiddleIndex(size: 10) Duration 100 1.362 0.031 1.318 1.455
ArrayBinarySearch_Int_MiddleIndex(size: 100) Duration 100 1.362 0.032 1.322 1.464
ArrayBinarySearch_Int_MiddleIndex(size: 1000) Duration 100 1.290 0.300 0.000 1.490
ArrayBinarySearch_Int_NotFoundAfter(size: 1) Duration 100 1.453 0.033 1.364 1.534
ArrayBinarySearch_Int_NotFoundAfter(size: 10) Duration 100 1.980 0.033 1.908 2.066
ArrayBinarySearch_Int_NotFoundAfter(size: 100) Duration 100 2.483 0.057 2.366 2.604
ArrayBinarySearch_Int_NotFoundAfter(size: 1000) Duration 100 3.007 0.070 2.873 3.353
ArrayBinarySearch_Int_NotFoundBefore(size: 1) Duration 100 1.421 0.038 1.336 1.488
ArrayBinarySearch_Int_NotFoundBefore(size: 10) Duration 100 1.737 0.055 1.615 2.067
ArrayBinarySearch_Int_NotFoundBefore(size: 100) Duration 100 2.140 0.059 2.032 2.274
ArrayBinarySearch_Int_NotFoundBefore(size: 1000) Duration 100 2.558 0.058 2.450 2.722
ArrayBinarySearch_String_FirstIndex(size: 1) Duration 100 24.152 0.279 23.505 24.870
ArrayBinarySearch_String_FirstIndex(size: 10) Duration 100 67.717 0.636 66.251 69.698
ArrayBinarySearch_String_FirstIndex(size: 100) Duration 79 127.637 1.291 125.175 132.090
ArrayBinarySearch_String_FirstIndex(size: 1000) Duration 54 187.029 2.363 183.460 197.605
ArrayBinarySearch_String_LastIndex(size: 1) Duration 100 24.109 0.260 23.394 24.659
ArrayBinarySearch_String_LastIndex(size: 10) Duration 100 90.462 0.870 88.901 95.927
ArrayBinarySearch_String_LastIndex(size: 100) Duration 67 149.617 1.113 147.428 152.731
ArrayBinarySearch_String_LastIndex(size: 1000) Duration 48 211.709 2.598 207.339 224.370
ArrayBinarySearch_String_MiddleIndex(size: 1) Duration 100 24.275 0.237 23.618 24.814
ArrayBinarySearch_String_MiddleIndex(size: 10) Duration 100 24.101 0.253 23.468 24.974
ArrayBinarySearch_String_MiddleIndex(size: 100) Duration 100 24.148 0.282 23.543 24.911
ArrayBinarySearch_String_MiddleIndex(size: 1000) Duration 100 24.288 0.355 23.565 25.233
ArrayBinarySearch_String_NotFoundAfter(size: 1) Duration 100 24.421 0.621 23.702 29.705
ArrayBinarySearch_String_NotFoundAfter(size: 10) Duration 100 77.455 0.670 76.127 79.343
ArrayBinarySearch_String_NotFoundAfter(size: 100) Duration 80 126.242 0.947 124.447 129.343
ArrayBinarySearch_String_NotFoundAfter(size: 1000) Duration 61 164.339 1.101 161.890 167.152
ArrayBinarySearch_String_NotFoundBefore(size: 1) Duration 100 12.681 0.198 12.326 13.212
ArrayBinarySearch_String_NotFoundBefore(size: 10) Duration 100 35.213 0.373 34.631 36.519
ArrayBinarySearch_String_NotFoundBefore(size: 100) Duration 100 62.672 0.991 60.915 67.299
ArrayBinarySearch_String_NotFoundBefore(size: 1000) Duration 100 91.516 0.746 89.792 93.891
SpanBinarySearch_Int_FirstIndex(size: 1) Duration 100 0.301 0.005 0.294 0.318
SpanBinarySearch_Int_FirstIndex(size: 10) Duration 100 0.674 0.023 0.623 0.744
SpanBinarySearch_Int_FirstIndex(size: 100) Duration 100 1.046 0.046 0.984 1.152
SpanBinarySearch_Int_FirstIndex(size: 1000) Duration 100 1.505 0.052 1.386 1.603
SpanBinarySearch_Int_LastIndex(size: 1) Duration 100 0.304 0.009 0.294 0.334
SpanBinarySearch_Int_LastIndex(size: 10) Duration 100 0.816 0.025 0.779 0.891
SpanBinarySearch_Int_LastIndex(size: 100) Duration 100 1.356 0.034 1.281 1.426
SpanBinarySearch_Int_LastIndex(size: 1000) Duration 100 1.885 0.051 1.782 1.969
SpanBinarySearch_Int_MiddleIndex(size: 1) Duration 100 0.294 0.012 0.278 0.340
SpanBinarySearch_Int_MiddleIndex(size: 10) Duration 100 0.302 0.005 0.295 0.329
SpanBinarySearch_Int_MiddleIndex(size: 100) Duration 100 0.294 0.013 0.278 0.334
SpanBinarySearch_Int_MiddleIndex(size: 1000) Duration 100 0.297 0.016 0.278 0.385
SpanBinarySearch_Int_NotFoundAfter(size: 1) Duration 100 0.334 0.012 0.323 0.404
SpanBinarySearch_Int_NotFoundAfter(size: 10) Duration 100 0.962 0.057 0.918 1.455
SpanBinarySearch_Int_NotFoundAfter(size: 100) Duration 100 1.383 0.031 1.326 1.477
SpanBinarySearch_Int_NotFoundAfter(size: 1000) Duration 100 1.908 0.049 1.817 2.076
SpanBinarySearch_Int_NotFoundBefore(size: 1) Duration 100 0.298 0.009 0.295 0.383
SpanBinarySearch_Int_NotFoundBefore(size: 10) Duration 100 0.573 0.013 0.561 0.647
SpanBinarySearch_Int_NotFoundBefore(size: 100) Duration 100 0.920 0.016 0.864 0.968
SpanBinarySearch_Int_NotFoundBefore(size: 1000) Duration 100 1.265 0.034 1.197 1.339
SpanBinarySearch_String_FirstIndex(size: 1) Duration 100 24.081 0.314 23.481 24.939
SpanBinarySearch_String_FirstIndex(size: 10) Duration 100 67.620 0.831 65.922 72.061
SpanBinarySearch_String_FirstIndex(size: 100) Duration 78 129.594 1.312 127.684 136.034
SpanBinarySearch_String_FirstIndex(size: 1000) Duration 55 183.044 1.672 179.561 187.654
SpanBinarySearch_String_LastIndex(size: 1) Duration 100 24.069 0.306 23.358 24.830
SpanBinarySearch_String_LastIndex(size: 10) Duration 100 88.475 0.671 87.047 90.305
SpanBinarySearch_String_LastIndex(size: 100) Duration 67 149.397 1.759 147.435 158.016
SpanBinarySearch_String_LastIndex(size: 1000) Duration 49 204.532 1.352 201.917 207.409
SpanBinarySearch_String_MiddleIndex(size: 1) Duration 100 24.192 0.384 23.390 25.623
SpanBinarySearch_String_MiddleIndex(size: 10) Duration 100 24.185 0.254 23.629 24.838
SpanBinarySearch_String_MiddleIndex(size: 100) Duration 100 24.245 0.336 23.557 25.424
SpanBinarySearch_String_MiddleIndex(size: 1000) Duration 100 24.136 0.288 23.532 25.513
SpanBinarySearch_String_NotFoundAfter(size: 1) Duration 100 24.172 0.286 23.562 24.985
SpanBinarySearch_String_NotFoundAfter(size: 10) Duration 100 76.572 0.599 75.079 78.241
SpanBinarySearch_String_NotFoundAfter(size: 100) Duration 82 123.173 1.227 121.331 129.054
SpanBinarySearch_String_NotFoundAfter(size: 1000) Duration 63 160.362 1.506 157.191 165.588
SpanBinarySearch_String_NotFoundBefore(size: 1) Duration 100 12.283 0.190 11.854 12.727
SpanBinarySearch_String_NotFoundBefore(size: 10) Duration 100 32.589 0.493 31.624 35.652
SpanBinarySearch_String_NotFoundBefore(size: 100) Duration 100 64.224 0.925 62.709 66.379
SpanBinarySearch_String_NotFoundBefore(size: 1000) Duration 100 88.981 0.749 87.475 91.942

@KrzysztofCwalina
Copy link
Member

Very nice results! The PR looks good.

ahsonkhan
ahsonkhan reviewed Dec 21, 2017
View reviewed changes
src/System.Memory/src/System/MemoryExtensions.cs Outdated
@@ -821,12 +821,6 @@ public static bool Overlaps<T>(this ReadOnlySpan<T> first, ReadOnlySpan<T> secon
/// <exception cref="T:System.ArgumentNullException">
/// <paramref name = "value" /> is <see langword="null"/> .
Copy link
Member

Choose a reason for hiding this comment

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

Do we throw when value is null or when comparer is null (or both)?

Copy link
Author

Choose a reason for hiding this comment

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

only the comparer I think since the comparer could be handling searching for null values. So it is a mistake in the comment here.

ahsonkhan
ahsonkhan approved these changes Dec 21, 2017
View reviewed changes
Copy link
Member

@ahsonkhan ahsonkhan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

otherwise, lgtm. tyvm!

@nietras
Copy link
Author

nietras commented Dec 22, 2017

@ahsonkhan fixed the xml comment, good catch :) A question, are these comments even used, most of the methods only have a summary, so wasn't sure what the convention was?

@ahsonkhan
Copy link
Member

@nietras, can you please resolve the merge conflict?

A question, are these comments even used, most of the methods only have a summary, so wasn't sure what the convention was?

The xml comments can be observed from VS IntelliSense as far as I can know,

@nietras
Copy link
Author

nietras commented Dec 30, 2017 •
edited
Loading

@ahsonkhan merged and resolved conflict.

@nietras
Copy link
Author

nietras commented Dec 30, 2017

@ahsonkhan I have a follow up question, a bit off topic. Is there any way to run specific performance tests and not all performance tests using the msbuild /t:BuildAndTest /p:Performance=true /p:ConfigurationGroup=Release /p:TargetOS=Windows_NT command? Perhaps using a trait or something? Could not find any docs or examples doing this?

@KrzysztofCwalina
Copy link
Member

@nietras, yes. See https://github.com/dotnet/corefxlab/blob/master/scripts/PerfHarness/run.bat. The perf:typenames parameter.

@nietras
Copy link
Author

nietras commented Jan 2, 2018

@KrzysztofCwalina thanks I'll have to see how this relates to the msbuild command. By the way, this PR is ready for merge.

@ahsonkhan
Copy link
Member

I have a follow up question, a bit off topic. Is there any way to run specific performance tests and not all performance tests using

The perf:typenames parameter.

corefx uses build0007 of xunit.performance (from May) while corefxlab uses a newer version:
https://github.com/dotnet/corefx/search?utf8=%E2%9C%93&q=XunitPerfAnalysisPackageVersion&type=

I think build0007 has support for that option, so it should work (was introduced around build0003 I believe).

Otherwise, as a workaround, you could comment out parts of the following while running perf tests locally:
https://github.com/dotnet/corefx/blob/master/src/System.Memory/tests/Performance/System.Memory.Performance.Tests.csproj#L11-L20

cc @jorive

@nietras
Copy link
Author

nietras commented Jan 2, 2018

@ahsonkhan how would I pass the perf:typenames parameter via msbuild command? Or do you then run it in a different way?

@jorive
Copy link
Member

jorive commented Jan 2, 2018

@ahsonkhan have you tried running this command: msbuild .\src\System.Memory\tests\Performance\System.Memory.Performance.Tests.csproj /t:BuildAndTest /p:Performance=true /p:ConfigurationGroup=Release /p:TargetOS=Windows_NT
It will build and test the specified project only.

@ahsonkhan
Copy link
Member

ahsonkhan commented Jan 2, 2018 •
edited
Loading

have you tried running this command

Yes, @jorive, that is what I do when I want to run all the System.Memory performance tests only.

Is there any way to run specific performance tests and not all performance tests

I am assuming that @nietras was looking for higher granularity, i.e. run specific subset of tests from the set of System.Memory perf tests. @nietras, please clarify if that was your intention.

how would I pass the perf:typenames parameter via msbuild command? Or do you then run it in a different way?

In corefxlab, we run it using the PerfHarness and dotnet cli (not msbuild) as described here:

@ahsonkhan ahsonkhan merged commit ae860b8 into dotnet:master Jan 2, 2018
@nietras nietras deleted the span-binarysearch-fixes branch January 3, 2018 08:53
@nietras
Copy link
Author

nietras commented Jan 3, 2018

assuming that @nietras was looking for higher granularity, i.e. run specific subset of tests from the set of System.Memory perf tests

@ahsonkhan @jorive exactly! So for example run just the tests for Perf_Span_BinarySearch in System.Memory.

we run it using the PerfHarness and dotnet cli (not msbuild)

Ok, so it is not possible using msbuild command. I'll have to try using PerfHarness then. Thanks for the merge!

@karelz karelz added this to the 2.1.0 milestone Jan 20, 2018
stephentoub pushed a commit that referenced this pull request Aug 6, 2019
@lambdageek @stephentoub
Move AssemblyLoadContext VM-invoked resolve methods to shared (#25989)
c6f5cee 
Share between CoreCLR and Mono

Mono began using a copy of these methods with
mono/mono#15946

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@nietras @ahsonkhan
Span: BinarySearch add Array baseline perf tests and remove TODO comm…
5eb19fc 
…ents (dotnet/corefx#25989)

* Add baseline Array BinarySearch perf tests, remove TODO comments.

* cleanup more TODOs

* fix xml comment


Commit migrated from dotnet/corefx@ae860b8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ahsonkhan ahsonkhan ahsonkhan approved these changes

@KrzysztofCwalina KrzysztofCwalina KrzysztofCwalina approved these changes

@joshfree joshfree Awaiting requested review from joshfree

@karelz karelz Awaiting requested review from karelz

Assignees

@ahsonkhan ahsonkhan

@KrzysztofCwalina KrzysztofCwalina

@nietras nietras

Labels
area-System.Memory
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nietras @KrzysztofCwalina @ahsonkhan @jorive @karelz