Skip to content

Add AVX2 support to IndexOfAnyValues - #78863

Merged
MihaZupan merged 1 commit into
dotnet:mainfrom
MihaZupan:indexofanyvalues-avx2
Jan 18, 2023
Merged

Add AVX2 support to IndexOfAnyValues#78863
MihaZupan merged 1 commit into
dotnet:mainfrom
MihaZupan:indexofanyvalues-avx2

Conversation

@MihaZupan

@MihaZupanMihaZupan commented Nov 26, 2022

Copy link
Copy Markdown
Member

Easier to review while ignoring whitespace

This change makes IndexOfAnyValues noticeably faster than a regular IndexOf(char) without #78861 :)

MethodToolchainLengthMeanErrorRatio
IndexOfAnyAsciiCharmain12.270 ns0.0145 ns1.00
IndexOfAnyAsciiCharpr12.279 ns0.0204 ns1.00
IndexOfAnyAsciiBytemain11.787 ns0.0053 ns1.00
IndexOfAnyAsciiBytepr11.788 ns0.0083 ns1.00
IndexOfAnyBytemain11.789 ns0.0050 ns1.00
IndexOfAnyBytepr11.790 ns0.0051 ns1.00
IndexOfAnyAsciiCharmain162.784 ns0.0247 ns1.00
IndexOfAnyAsciiCharpr162.782 ns0.0252 ns1.00
IndexOfAnyAsciiBytemain162.696 ns0.0117 ns1.00
IndexOfAnyAsciiBytepr162.364 ns0.0084 ns0.88
IndexOfAnyBytemain163.887 ns0.0196 ns1.00
IndexOfAnyBytepr163.488 ns0.0214 ns0.90
IndexOfAnyAsciiCharmain173.348 ns0.0125 ns1.00
IndexOfAnyAsciiCharpr173.103 ns0.0639 ns0.94
IndexOfAnyAsciiBytemain173.122 ns0.0109 ns1.00
IndexOfAnyAsciiBytepr173.061 ns0.0124 ns0.98
IndexOfAnyBytemain175.465 ns0.0762 ns1.00
IndexOfAnyBytepr174.102 ns0.0258 ns0.75
IndexOfAnyAsciiCharmain323.381 ns0.0098 ns1.00
IndexOfAnyAsciiCharpr323.042 ns0.0095 ns0.90
IndexOfAnyAsciiBytemain323.261 ns0.0063 ns1.00
IndexOfAnyAsciiBytepr323.074 ns0.0138 ns0.94
IndexOfAnyBytemain325.538 ns0.0131 ns1.00
IndexOfAnyBytepr324.149 ns0.0197 ns0.75
IndexOfAnyAsciiCharmain100060.429 ns0.2208 ns1.00
IndexOfAnyAsciiCharpr100034.636 ns0.1453 ns0.57
IndexOfAnyAsciiBytemain100049.191 ns0.1576 ns1.00
IndexOfAnyAsciiBytepr100034.826 ns0.4630 ns0.71
IndexOfAnyBytemain100083.976 ns0.2974 ns1.00
IndexOfAnyBytepr100045.647 ns0.1477 ns0.54
IndexOfAnyAsciiCharmain1000005,865.958 ns23.1353 ns1.00
IndexOfAnyAsciiCharpr1000003,880.814 ns35.5076 ns0.66
IndexOfAnyAsciiBytemain1000004,753.754 ns15.7070 ns1.00
IndexOfAnyAsciiBytepr1000003,215.513 ns10.5250 ns0.68
IndexOfAnyBytemain1000007,691.448 ns38.5081 ns1.00
IndexOfAnyBytepr1000004,488.097 ns41.8751 ns0.58

Chars

MethodLengthMeanErrorStdDev
IndexOfAnyAsciiChar1000003.936 us0.0525 us0.0491 us
IndexOf1000004.925 us0.0186 us0.0174 us
IndexOfAny2Values1000005.754 us0.0196 us0.0174 us
IndexOfAny3Values1000006.161 us0.0257 us0.0228 us
IndexOfAny4Values1000007.011 us0.0162 us0.0135 us
IndexOfAny5Values1000007.743 us0.0363 us0.0303 us
IndexOfAnyInRange_Char1000005.196 us0.0143 us0.0120 us

Bytes

MethodLengthMeanErrorStdDev
IndexOfAnyAsciiByte1000003.189 us0.0171 us0.0151 us
IndexOfAnyByte1000004.484 us0.0259 us0.0229 us
IndexOf1000001.971 us0.0186 us0.0174 us
IndexOfAny2Values1000002.233 us0.0093 us0.0078 us
IndexOfAny3Values1000002.628 us0.0081 us0.0076 us
IndexOfAny4Values1000002.929 us0.0155 us0.0145 us
IndexOfAny5Values1000003.270 us0.0102 us0.0090 us
IndexOfAnyInRange_Byte1000002.187 us0.0090 us0.0084 us

@MihaZupanMihaZupan added this to the 8.0.0 milestone Nov 26, 2022
@MihaZupanMihaZupan self-assigned this Nov 26, 2022
@ghost

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

Easier to review while ignoring whitespace

This change makes IndexOfAnyValues noticeably faster than a regular IndexOf(char) without #78861 :)

MethodToolchainLengthMeanErrorRatio
IndexOfAnyAsciiCharmain12.270 ns0.0145 ns1.00
IndexOfAnyAsciiCharpr12.279 ns0.0204 ns1.00
IndexOfAnyAsciiBytemain11.787 ns0.0053 ns1.00
IndexOfAnyAsciiBytepr11.788 ns0.0083 ns1.00
IndexOfAnyBytemain11.789 ns0.0050 ns1.00
IndexOfAnyBytepr11.790 ns0.0051 ns1.00
IndexOfAnyAsciiCharmain162.784 ns0.0247 ns1.00
IndexOfAnyAsciiCharpr162.782 ns0.0252 ns1.00
IndexOfAnyAsciiBytemain162.696 ns0.0117 ns1.00
IndexOfAnyAsciiBytepr162.364 ns0.0084 ns0.88
IndexOfAnyBytemain163.887 ns0.0196 ns1.00
IndexOfAnyBytepr163.488 ns0.0214 ns0.90
IndexOfAnyAsciiCharmain173.348 ns0.0125 ns1.00
IndexOfAnyAsciiCharpr173.103 ns0.0639 ns0.94
IndexOfAnyAsciiBytemain173.122 ns0.0109 ns1.00
IndexOfAnyAsciiBytepr173.061 ns0.0124 ns0.98
IndexOfAnyBytemain175.465 ns0.0762 ns1.00
IndexOfAnyBytepr174.102 ns0.0258 ns0.75
IndexOfAnyAsciiCharmain323.381 ns0.0098 ns1.00
IndexOfAnyAsciiCharpr323.042 ns0.0095 ns0.90
IndexOfAnyAsciiBytemain323.261 ns0.0063 ns1.00
IndexOfAnyAsciiBytepr323.074 ns0.0138 ns0.94
IndexOfAnyBytemain325.538 ns0.0131 ns1.00
IndexOfAnyBytepr324.149 ns0.0197 ns0.75
IndexOfAnyAsciiCharmain100060.429 ns0.2208 ns1.00
IndexOfAnyAsciiCharpr100034.636 ns0.1453 ns0.57
IndexOfAnyAsciiBytemain100049.191 ns0.1576 ns1.00
IndexOfAnyAsciiBytepr100034.826 ns0.4630 ns0.71
IndexOfAnyBytemain100083.976 ns0.2974 ns1.00
IndexOfAnyBytepr100045.647 ns0.1477 ns0.54
IndexOfAnyAsciiCharmain1000005,865.958 ns23.1353 ns1.00
IndexOfAnyAsciiCharpr1000003,880.814 ns35.5076 ns0.66
IndexOfAnyAsciiBytemain1000004,753.754 ns15.7070 ns1.00
IndexOfAnyAsciiBytepr1000003,215.513 ns10.5250 ns0.68
IndexOfAnyBytemain1000007,691.448 ns38.5081 ns1.00
IndexOfAnyBytepr1000004,488.097 ns41.8751 ns0.58

Chars

MethodLengthMeanErrorStdDev
IndexOfAnyAsciiChar1000003.936 us0.0525 us0.0491 us
IndexOf1000004.925 us0.0186 us0.0174 us
IndexOfAny2Values1000005.754 us0.0196 us0.0174 us
IndexOfAny3Values1000006.161 us0.0257 us0.0228 us
IndexOfAny4Values1000007.011 us0.0162 us0.0135 us
IndexOfAny5Values1000007.743 us0.0363 us0.0303 us
IndexOfAnyInRange_Char1000005.196 us0.0143 us0.0120 us

Bytes

MethodLengthMeanErrorStdDev
IndexOfAnyAsciiByte1000003.189 us0.0171 us0.0151 us
IndexOfAnyByte1000004.484 us0.0259 us0.0229 us
IndexOf1000001.971 us0.0186 us0.0174 us
IndexOfAny2Values1000002.233 us0.0093 us0.0078 us
IndexOfAny3Values1000002.628 us0.0081 us0.0076 us
IndexOfAny4Values1000002.929 us0.0155 us0.0145 us
IndexOfAny5Values1000003.270 us0.0102 us0.0090 us
IndexOfAnyInRange_Byte1000002.187 us0.0090 us0.0084 us
Author:MihaZupan
Assignees:MihaZupan
Labels:

area-System.Memory, tenet-performance

Milestone:8.0.0

@MihaZupan
MihaZupan requested a review from a teamJanuary 4, 2023 23:35
@dakersnar

Copy link
Copy Markdown
Contributor

This change makes IndexOfAnyValues noticeably faster than a regular IndexOf(char) without #78861 :)

For context, does this PR have any relation to the changes made in #78861, or is this a completely independent optimization?

@MihaZupan

Copy link
Copy Markdown
MemberAuthor

Completely independent, I just happened to have worked on both at a similar point in time.

@MihaZupan

Copy link
Copy Markdown
MemberAuthor

Any thoughts on this PR @dotnet/area-system-memory?

@dakersnar

Copy link
Copy Markdown
Contributor

Sorry, this has been on my TODO list. Can you provide a similar explanation to what you did here: #78861 (comment)?

@MihaZupan

Copy link
Copy Markdown
MemberAuthor

This file (IndexOfAnyAsciiSearcher.cs) already contains a bunch of vectorization code for Ssse3 and Arm64 through Vector128.

This PR effectively copy-pastes the existing Vector128 logic to add Vector256 paths. Everything is pretty much the same, just processing 2x chars/bytes per iteration.

Taking IndexOfAnyVectorized as an example. Currently it's written as

if(searchSpaceLength>16){// Process the input with 2 Vector128s (16 chars) at a time.}// Process the last 1-16 chars with 2 overlapped Vector128s.returnNotFound;

and after this PR, it's written as

if(searchSpaceLength>16){if(Avx2.IsSupported){if(searchSpaceLength>32){// Process the input with 2 Vector256s (32 chars) at a time.}// Process the last 1-32 chars with 2 overlapped Vector256s.returnNotFound;}else{// Process the input with 2 Vector128s (16 chars) at a time.}}// Process the last 1-16 chars with 2 overlapped Vector128s.returnNotFound;

@dakersnardakersnar left a comment

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.

Looks good and thank you for the explanation. I assume all the helper methods are just copy pasted from the Vector128 implementations?

{
private const int MaxNeedleLength = 10;
private const int MaxHaystackLength = 40;
private const int MaxHaystackLength = 100;

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.

Where does this number come from?

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.

These are used by the test when running a few million random inputs through the implementation to check random edge cases.
This number should be large enough so that we test all sorts of code paths - e.g. 100 is enough for

// Process the input with 2 Vector256s (32 chars) at a time.+// Process the input with 2 Vector256s (32 chars) at a time.+// Process the last 1-32 chars with 2 overlapped Vector256s.

so if the "2 Vector256s at a time" step left something broken, we'd catch it.

On the other hand it doesn't make sense to increase it too much as we'd just end up testing fewer variants of short inputs - and there's nothing in the code that would behave fundamentally differently for inputs of 1k vs 10k characters.

@MihaZupan

MihaZupan commented Jan 18, 2023

Copy link
Copy Markdown
MemberAuthor

I assume all the helper methods are just copy pasted from the Vector128 implementations?

Yes*, with the exception that we call this FixUpPackedVector256Mask helper method that accounts for how AVX2 does packing (it's more like 2x 128bit instead of 1x 256bit). We do the same thing in PackedSpanHelpers.

@MihaZupan

Copy link
Copy Markdown
MemberAuthor

The build failure is known according to Build Analysis

@MihaZupan
MihaZupan merged commit 4e0195e into dotnet:mainJan 18, 2023
mdh1418 pushed a commit to mdh1418/runtime that referenced this pull request Jan 24, 2023
@ghostghost locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Memorytenet-performancePerformance related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@MihaZupan@dakersnar