This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Improve pref of Array.IndexOf() for certain T.
#24293
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in dotnet#20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`.
EgorBo
reviewed
Apr 28, 2019
| Unsafe.As<T, long>(ref value), | ||
| count); | ||
| return (result >= 0 ? startIndex : 0) + result; | ||
| } |
There was a problem hiding this comment.
I wonder - is it possible to replace it all with return new Span<T>(array, startIndex, count).IndexOf(value); to re-use that MemoryExtensions code with IsBitwiseEquatable
There was a problem hiding this comment.
MemoryExtensions.IndexOf<T>(Span<T>, T) requires that T implement IEquatable<T>, and while we know that all types for which IsBitwiseEquatable<T>() currently returns true satisfy that requirement, there is no way to tell that to the compiler.
|
Do you have any numbers that show how much this change improved the performance? |
jkotas
approved these changes
Apr 29, 2019
GrabYourPitchforks
approved these changes
Apr 29, 2019
|
I tested using the same logic as the For example: [Benchmark]
public int ArrayIndexOf()
{
int result = 0;
var values = _values;
var found = _found;
for (int i = 0; i < found.Length; i++)
result ^= Array.IndexOf(values, found[i]);
return result;
}BenchmarkDotNet=v0.11.5, OS=Windows 10.0.17134.706 (1803/April2018Update/Redstone4)
Intel Core i7-8700K CPU 3.70GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
Frequency=3609369 Hz, Resolution=277.0567 ns, Timer=TSC
.NET Core SDK=3.0.100-preview5-011281
[Host] : .NET Core 3.0.0-preview5-27613-06 (CoreCLR 4.6.27613.71, CoreFX 4.700.19.21214), 64bit RyuJIT
Job-JLWIJH : .NET Core ? (CoreCLR 4.6.27620.0, CoreFX 4.700.19.21911), 64bit RyuJIT
Job-OBNZOS : .NET Core ? (CoreCLR 4.6.27620.0, CoreFX 4.700.19.21911), 64bit RyuJIT
IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15
WarmupCount=1
|
|
The CI failures are unrelated. I wouldn't block this merge waiting for CI to go green. |
Dotnet-GitSync-Bot
pushed a commit
to Dotnet-GitSync-Bot/corefx
that referenced
this pull request
Apr 30, 2019
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in #20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Signed-off-by: dotnet-bot <[email protected]>
stephentoub
pushed a commit
to dotnet/corefx
that referenced
this pull request
Apr 30, 2019
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in #20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Signed-off-by: dotnet-bot <[email protected]>
|
Thanks @dschinde for your contribution! :) |
Dotnet-GitSync-Bot
pushed a commit
to Dotnet-GitSync-Bot/corert
that referenced
this pull request
Apr 30, 2019
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in #20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Signed-off-by: dotnet-bot <[email protected]>
Dotnet-GitSync-Bot
pushed a commit
to Dotnet-GitSync-Bot/mono
that referenced
this pull request
Apr 30, 2019
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in mono#20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Signed-off-by: dotnet-bot <[email protected]>
jkotas
pushed a commit
to dotnet/corert
that referenced
this pull request
Apr 30, 2019
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in #20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Signed-off-by: dotnet-bot <[email protected]>
marek-safar
pushed a commit
to mono/mono
that referenced
this pull request
Apr 30, 2019
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in #20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Signed-off-by: dotnet-bot <[email protected]>
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
) Applies changes to `Array.IndexOf()` and `Array.LastIndexOf()` similar to the changes made in dotnet/coreclr#20855, so that types other than `byte` and `char` can use use the fast vectorized path. Also allows 32-bit and 64-bit types for which `RuntimeHelpers.IsBitwiseEquatable<T>()` returns `true` to use the faster implementation of `IndexOf` and `LastIndexOf` from `MemoryExtensions`. Commit migrated from dotnet/coreclr@2551753
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applies changes to
Array.IndexOf()andArray.LastIndexOf()similarto the changes made in #20855, so that types other than
byteandcharcan use use the fast vectorized path.Also allows 32-bit and 64-bit types for which
RuntimeHelpers.IsBitwiseEquatable<T>()returnstrueto use thefaster implementation of
IndexOfandLastIndexOffromMemoryExtensions.