Skip to content

Port SpanHelpers.SequenceCompareTo(ref byte, int, ref byte, int) to Vector128/256 - #73475

Merged
adamsitnik merged 7 commits into
dotnet:mainfrom
adamsitnik:spanSequenceCompareTo
Oct 10, 2022
Merged

Port SpanHelpers.SequenceCompareTo(ref byte, int, ref byte, int) to Vector128/256#73475
adamsitnik merged 7 commits into
dotnet:mainfrom
adamsitnik:spanSequenceCompareTo

Conversation

@adamsitnik

@adamsitnikadamsitnik commented Aug 5, 2022

Copy link
Copy Markdown
Member

For Arm64 we have a +- 20% improvement, mostly due to the fact that this code has not been optimized for ARM64.

BenchmarkDotNet=v0.13.1.1828-nightly, OS=ubuntu 20.04
Unknown processor
.NET SDK=7.0.100-rc.1.22405.1
[Host] : .NET 7.0.0 (7.0.22.40308), Arm64 RyuJIT AdvSIMD
Job-PHNJLQ : .NET 7.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
Job-FYBYWI : .NET 7.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
MethodToolchainSizeMeanRatio
SequenceCompareTo/PR/corerun51243.273 ns0.83
SequenceCompareTo/main/corerun51252.073 ns1.00
SequenceCompareToDifferent/PR/corerun5125.393 ns0.82
SequenceCompareToDifferent/main/corerun5126.547 ns1.00

For x64 the performance is on par for both AVX2 and AVX.

BenchmarkDotNet=v0.13.1.1828-nightly, OS=Windows 11 (10.0.22000.795/21H2)
AMD Ryzen Threadripper PRO 3945WX 12-Cores, 1 CPU, 24 logical and 12 physical cores
.NET SDK=7.0.100-preview.7.22377.5
[Host] : .NET 7.0.0 (7.0.22.37506), X64 RyuJIT AVX2
Job-ZYEZPW : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT AVX
Job-DJUBNM : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT AXV
EnvironmentVariables=COMPlus_EnableAVX2=0
MethodToolchainSizeMeanRatio
SequenceCompareTo\PR\corerun.exe51220.099 ns1.03
SequenceCompareTo\baseline\corerun.exe51219.485 ns1.00
SequenceCompareToDifferent\7.0.0\corerun.exe5124.565 ns0.98
SequenceCompareToDifferent\baseline\corerun.exe5124.671 ns1.00

contributes to #64451

@adamsitnikadamsitnik added this to the 7.0.0 milestone Aug 5, 2022
@ghostghost assigned adamsitnikAug 5, 2022
@ghost

ghost commented Aug 5, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

Issue Details

For Arm64 we have a +- 20% improvement, mostly due to the fact that this code has not been optimized for ARM64.

BenchmarkDotNet=v0.13.1.1828-nightly, OS=ubuntu 20.04
Unknown processor
.NET SDK=7.0.100-rc.1.22405.1
[Host] : .NET 7.0.0 (7.0.22.40308), Arm64 RyuJIT AdvSIMD
Job-PHNJLQ : .NET 7.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
Job-FYBYWI : .NET 7.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
MethodToolchainSizeMeanRatio
SequenceCompareTo/PR/corerun51243.273 ns0.83
SequenceCompareTo/main/corerun51252.073 ns1.00
SequenceCompareToDifferent/PR/corerun5125.393 ns0.82
SequenceCompareToDifferent/main/corerun5126.547 ns1.00

For x64 the performance is on par for both AVX2 and AVX.

BenchmarkDotNet=v0.13.1.1828-nightly, OS=Windows 11 (10.0.22000.795/21H2)
AMD Ryzen Threadripper PRO 3945WX 12-Cores, 1 CPU, 24 logical and 12 physical cores
.NET SDK=7.0.100-preview.7.22377.5
[Host] : .NET 7.0.0 (7.0.22.37506), X64 RyuJIT AVX2
Job-ZYEZPW : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT AVX
Job-DJUBNM : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT AXV
EnvironmentVariables=COMPlus_EnableAVX2=0
MethodToolchainSizeMeanRatio
SequenceCompareTo\PR\corerun.exe51220.099 ns1.03
SequenceCompareTo\baseline\corerun.exe51219.485 ns1.00
SequenceCompareToDifferent\7.0.0\corerun.exe5124.565 ns0.98
SequenceCompareToDifferent\baseline\corerun.exe5124.671 ns1.00

contributes to #64451

Author:adamsitnik
Assignees:-
Labels:

area-System.Memory, tenet-performance

Milestone:7.0.0

@adamsitnikadamsitnik mentioned this pull request Aug 5, 2022
Open
75 tasks
This was referenced Aug 5, 2022
Closed
Closed
@adamsitnik

adamsitnik commented Aug 11, 2022

Copy link
Copy Markdown
MemberAuthor

@tannergooding@EgorBo sorry to bother you guys, but it would be really nice to get this merged in 7 ;)

stephentoub
stephentoub reviewed Aug 11, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/SpanHelpers.Byte.cs
}

goto Difference;
goto BytewiseCheck;

@stephentoubstephentoubAug 11, 2022

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.

Isn't this replacing the vectorized detection of which element in the vector differed with a linear walk through all bytes in the vector? If so, did you validate the perf impact of this on inputs smaller than 512 elements? I'm surprised this wouldn't result in regressions.

@adamsitnikadamsitnikOct 7, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@stephentoub please excuse me for the delay. That is true, as Vector128 code path is also executed for arm64, where ExtractMostSignificantBits is expensive. I've used BytewiseCheck which was so far used by Vector<T> code path and its perf is OK.

I've added benchmarks for smaller collection sizes, synced the fork and re-run them.

x64 AVX2 (Vector256)

It's more or less on par.

Details
BenchmarkDotNet=v0.13.2.1937-nightly, OS=Windows 11 (10.0.22000.978/21H2)
AMD Ryzen Threadripper PRO 3945WX 12-Cores, 1 CPU, 24 logical and 12 physical cores
.NET SDK=7.0.100-rtm.22506.1
[Host] : .NET 7.0.0 (7.0.22.48010), X64 RyuJIT AVX2
main : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2
pr : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2
MethodJobToolchainSizeMeanRatio
SequenceCompareTomain\main\corerun.exe87.522 ns1.00
SequenceCompareTopr\prSync\corerun.exe87.461 ns0.99
SequenceCompareToDifferentmain\main\corerun.exe84.344 ns1.00
SequenceCompareToDifferentpr\prSync\corerun.exe84.230 ns0.97
SequenceCompareTomain\main\corerun.exe323.889 ns1.00
SequenceCompareTopr\prSync\corerun.exe323.869 ns0.99
SequenceCompareToDifferentmain\main\corerun.exe324.651 ns1.00
SequenceCompareToDifferentpr\prSync\corerun.exe324.603 ns0.99
SequenceCompareTomain\main\corerun.exe644.426 ns1.00
SequenceCompareTopr\prSync\corerun.exe644.446 ns1.00
SequenceCompareToDifferentmain\main\corerun.exe644.314 ns1.00
SequenceCompareToDifferentpr\prSync\corerun.exe644.332 ns1.00
SequenceCompareTomain\main\corerun.exe1285.506 ns1.00
SequenceCompareTopr\prSync\corerun.exe1285.558 ns1.01
SequenceCompareToDifferentmain\main\corerun.exe1284.337 ns1.00
SequenceCompareToDifferentpr\prSync\corerun.exe1284.321 ns1.00
SequenceCompareTomain\main\corerun.exe51211.758 ns1.00
SequenceCompareTopr\prSync\corerun.exe51211.694 ns0.99
SequenceCompareToDifferentmain\main\corerun.exe5124.324 ns1.00
SequenceCompareToDifferentpr\prSync\corerun.exe5124.321 ns1.00

x64 AVX (Vector128)

It's 2-3% slower, but it translates to just +- 0.2ns.

Details
BenchmarkDotNet=v0.13.2.1937-nightly, OS=Windows 11 (10.0.22000.978/21H2)
AMD Ryzen Threadripper PRO 3945WX 12-Cores, 1 CPU, 24 logical and 12 physical cores
.NET SDK=7.0.100-rtm.22506.1
[Host] : .NET 7.0.0 (7.0.22.48010), X64 RyuJIT AVX2
main : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX
pr : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX
EnvironmentVariables=COMPlus_EnableAVX2=0
MethodJobSizeMeanRatio
SequenceCompareTomain87.058 ns1.00
SequenceCompareTopr86.696 ns0.95
SequenceCompareToDifferentmain84.088 ns1.00
SequenceCompareToDifferentpr84.099 ns1.00
SequenceCompareTomain324.343 ns1.00
SequenceCompareTopr324.273 ns0.98
SequenceCompareToDifferentmain324.323 ns1.00
SequenceCompareToDifferentpr324.414 ns1.02
SequenceCompareTomain645.322 ns1.00
SequenceCompareTopr645.383 ns1.01
SequenceCompareToDifferentmain644.316 ns1.00
SequenceCompareToDifferentpr644.444 ns1.03
SequenceCompareTomain1286.987 ns1.00
SequenceCompareTopr1287.229 ns1.03
SequenceCompareToDifferentmain1284.307 ns1.00
SequenceCompareToDifferentpr1284.493 ns1.04
SequenceCompareTomain51218.724 ns1.00
SequenceCompareTopr51219.032 ns1.02
SequenceCompareToDifferentmain5124.302 ns1.00
SequenceCompareToDifferentpr5124.451 ns1.03

Arm64 AdvSimd (Vector128)

For cases where the inputs are different the perf remains the same, but we can observe a nice boost for equal imputs >= 8 elements (20% to even x3)

Details
BenchmarkDotNet=v0.13.2.1937-nightly, OS=ubuntu 20.04
Unknown processor
.NET SDK=7.0.100-rtm.22506.1
[Host] : .NET 7.0.0 (7.0.22.48010), Arm64 RyuJIT AdvSIMD
main : .NET 8.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
pr : .NET 8.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
LaunchCount=9 MemoryRandomization=True
MethodJobSizeMedian
SequenceCompareTomain86.551 ns
SequenceCompareTopr86.547 ns
SequenceCompareToDifferentmain82.696 ns
SequenceCompareToDifferentpr82.696 ns
SequenceCompareTomain3212.399 ns
SequenceCompareTopr323.851 ns
SequenceCompareToDifferentmain322.697 ns
SequenceCompareToDifferentpr322.696 ns
SequenceCompareTomain6415.863 ns
SequenceCompareTopr645.776 ns
SequenceCompareToDifferentmain644.090 ns
SequenceCompareToDifferentpr643.274 ns
SequenceCompareTomain12823.466 ns
SequenceCompareTopr12812.219 ns
SequenceCompareToDifferentmain1282.696 ns
SequenceCompareToDifferentpr1282.696 ns
SequenceCompareTomain51259.817 ns
SequenceCompareTopr51248.528 ns
SequenceCompareToDifferentmain5122.698 ns
SequenceCompareToDifferentpr5122.696 ns

@adamsitnikadamsitnik modified the milestones: 7.0.0, 8.0.0Oct 7, 2022
@build-analysisbuild-analysisBot mentioned this pull request Oct 7, 2022
Closed
@adamsitnik
adamsitnik merged commit 91ae19b into dotnet:mainOct 10, 2022
This was referenced Oct 11, 2022
Closed
Open
@ghostghost locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tannergoodingtannergoodingtannergooding approved these changes
@EgorBoEgorBoAwaiting requested review from EgorBo
@stephentoubstephentoubAwaiting requested review from stephentoub

Assignees

@adamsitnikadamsitnik

Labels

area-System.Memorytenet-performancePerformance related issue

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@adamsitnik@stephentoub@tannergooding