IndexOf/SequenceEqual methods for Span. #15080
Conversation
These are requested for the parsing libraries. There will also be ReadOnlySpan<> versions as well as char/byte-specific overloads (that bypass IEquatable.Equals() in favor of "==") but most or all of that will be copy-paste of these once we're happy with the generic versions.
Would it be better for byte/char versions to find pointer alignment, and read and compare machine word values at least for the IndexOf overloads that take ROS value? |
| public static int IndexOf<T>(this Span<T> span, T value) | ||
| where T:struct, IEquatable<T> | ||
| { | ||
| return SpanHelpers.IndexOf<T>(ref span.DangerousGetPinnableReference(), value, 0, span.Length); |
There was a problem hiding this comment.
Why do we have the actual implementation in separate method and not here?
There was a problem hiding this comment.
The helper will also be used for the ROSpan versions. Also, the helpers use each other (IndexOf of sequence uses IndexOf of value & SequenceEquals).
|
I expected that’s where the “most” qualifier comes in…
|
| @@ -83,6 +83,9 @@ public static class SpanExtensions | |||
| [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] public static System.ReadOnlySpan<byte> AsBytes<T>(this ReadOnlySpan<T> source) where T : struct { throw null; } | |||
| [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] public static System.Span<TTo> NonPortableCast<TFrom, TTo>(this System.Span<TFrom> source) where TFrom : struct where TTo : struct { throw null; } | |||
| [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] public static System.ReadOnlySpan<TTo> NonPortableCast<TFrom, TTo>(this System.ReadOnlySpan<TFrom> source) where TFrom : struct where TTo : struct { throw null; } | |||
| [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] public static int IndexOf<T>(this Span<T> span, T value) where T:struct, IEquatable<T> { throw null; } | |||
There was a problem hiding this comment.
The aggressive inlining attribute does not make sense in reference assemblies. Can we remove them instead from here?
There was a problem hiding this comment.
I'm ok with removing these, but they'll come back if someone uses genapi to regenerate the file...
| } | ||
| } | ||
|
|
||
| public static int IndexOf<T>(ref T searchSpace, T value, int start, int length) |
There was a problem hiding this comment.
I would remove the start argument, and instead made the caller the add it to searchSpace.
There was a problem hiding this comment.
I added the "start" argument because saved me an extra addition in the other IndexOf() helper (i.e. without it, I not only have to adjust searchSpace going in but adjust "index" coming out since it won't have the prior index value baked into it.)
Plus, I figure there's a 50/50 chance someone will want a public overload of IndexOf with a "start" parameter eventually...
There was a problem hiding this comment.
I think it is unfortunate to have the useless extra argument for the leanest IndexOf method. Would it make sense to have another tiny ForceInline helper that adds/subtracts the start for the cases where you have non-zero start?
There was a problem hiding this comment.
There's just one the case right now, so don't need a helper yet. I've restored the version I had before the "start" argument - on the principle that the IndexOf(T) is more important perf-wise.
|
LGTM modulo comments. |
|
looks good. |
ghost
merged commit 
* IndexOf/SequenceEqual methods for Span. These are requested for the parsing libraries. There will also be ReadOnlySpan<> versions as well as char/byte-specific overloads (that bypass IEquatable.Equals() in favor of "==") but most or all of that will be copy-paste of these once we're happy with the generic versions. * Remove the inlining attributes from the ref assembly. * Remove "start" argument. Commit migrated from dotnet/corefx@5d46a72
These are requested for the parsing libraries.
There will also be ReadOnlySpan<> versions as
well as char/byte-specific overloads (that
bypass IEquatable.Equals() in favor of "==")
but most or all of that will be copy-paste of
these once we're happy with the generic versions.