Add ContainsAny{Except} #87621

MihaZupan · 2023-06-15T15:26:57Z

Contributes to #86528 (SearchValues<string> and -WhiteSpace overloads remain as not yet implemented)

This PR adds ContainsAny as a helper around IndexOfAny >= 0.
I left out potential optimizations (e.g. dedicated Contains paths for SearchValues) for future PRs.

dotnet-issue-labeler · 2023-06-15T15:27:06Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2023-06-15T15:27:13Z

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

Issue Details

Contributes to #86528 (SearchValues<string> and -WhiteSpace overloads remain as not yet implemented)

This PR adds ContainsAny as a helper around IndexOfAny >= 0.
I left out potential optimizations (e.g. dedicated Contains paths for SearchValues) for future PRs.

Author:	MihaZupan
Assignees:	MihaZupan
Labels:	`area-System.Memory`
Milestone:	8.0.0

stephentoub · 2023-06-15T15:39:53Z

src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs

@@ -401,6 +396,192 @@ public static ReadOnlyMemory<char> AsMemory(this string? text, Range range)
            return SpanHelpers.Contains(ref MemoryMarshal.GetReference(span), value, span.Length);
        }

+        /// <inheritdoc cref="ContainsAny{T}(ReadOnlySpan{T}, T, T)"/>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]


Are all of these AggressiveInlinings actually needed?

Probably not everywhere, but it felt like they shouldn't hurt anything here. The intent at least is for these to be inlined such that anything interesting in IndexOfAny can be removed after inlining.

It'd be a bit sad if the JIT gave up on any of these because of the extra indirection.

@EgorBo, what should our approach here be in general?

I'd be surprised to see it non-inlined, although, this will never be inlined even with the Aggressive attribute for shared T due to runtime lookup

Do we want to be in the habit of annotating everything as AggressiveInlining we want inlined even if should be anyway?

Do we want to be in the habit of annotating everything as AggressiveInlining we want inlined even if should be anyway?

I am not a fan of that approach as it offends the jit 😄 although, we have other runtimes. I can probably run some script to find methods <= 16 bytes of IL with AggressiveInlining just out of curiosity (JIT always inlines methods <= 16 bytes of IL so the attribute is basically no-op)

E.g. I think in Java they don't even have such an attribute!

src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs

src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs

stephentoub

Thanks

) * Update dependencies from https://github.com/dotnet/aspnetcore build 20230621.15 Microsoft.AspNetCore.App.Runtime.win-x64 , Microsoft.AspNetCore.Mvc.Testing , Microsoft.AspNetCore.TestHost , Microsoft.Extensions.Caching.StackExchangeRedis , Microsoft.Extensions.Diagnostics.HealthChecks , Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions , Microsoft.Extensions.Features , Microsoft.Extensions.Http.Polly , Microsoft.Extensions.ObjectPool From Version 8.0.0-preview.6.23315.13 -> To Version 8.0.0-preview.6.23321.15 Dependency coherency updates Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Abstractions,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Configuration.Binder,Microsoft.Extensions.Configuration.CommandLine,Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Configuration,Microsoft.Extensions.DependencyInjection.Abstractions,Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Hosting.Abstractions,Microsoft.Extensions.Hosting,Microsoft.Extensions.Http,Microsoft.Extensions.Logging.Abstractions,Microsoft.Extensions.Logging.Configuration,Microsoft.Extensions.Logging.Console,Microsoft.Extensions.Logging,Microsoft.Extensions.Options.ConfigurationExtensions,Microsoft.Extensions.Options.DataAnnotations,Microsoft.Extensions.Options,Microsoft.Extensions.Primitives,System.Collections.Immutable,System.Configuration.ConfigurationManager,System.Diagnostics.DiagnosticSource,System.Diagnostics.PerformanceCounter,System.IO.Hashing,System.Net.Http.Json,System.Security.Cryptography.Pkcs,System.Security.Cryptography.Xml,System.Text.Encodings.Web,System.Text.Json,System.Runtime.Caching From Version 8.0.0-preview.6.23314.15 -> To Version 8.0.0-preview.6.23318.9 (parent: Microsoft.AspNetCore.App.Runtime.win-x64 * Bump the SDK to resolve the break caused by dotnet/runtime#87621 * Update local copy of ExperimentalAttribute per dotnet/runtime#85444 * Update dependencies from https://github.com/dotnet/aspnetcore build 20230623.8 Microsoft.AspNetCore.App.Runtime.win-x64 , Microsoft.AspNetCore.Mvc.Testing , Microsoft.AspNetCore.TestHost , Microsoft.Extensions.Caching.StackExchangeRedis , Microsoft.Extensions.Diagnostics.HealthChecks , Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions , Microsoft.Extensions.Features , Microsoft.Extensions.Http.Polly , Microsoft.Extensions.ObjectPool From Version 8.0.0-preview.6.23315.13 -> To Version 8.0.0-preview.6.23323.8 Dependency coherency updates Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Abstractions,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Configuration.Binder,Microsoft.Extensions.Configuration.CommandLine,Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Configuration,Microsoft.Extensions.DependencyInjection.Abstractions,Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Hosting.Abstractions,Microsoft.Extensions.Hosting,Microsoft.Extensions.Http,Microsoft.Extensions.Logging.Abstractions,Microsoft.Extensions.Logging.Configuration,Microsoft.Extensions.Logging.Console,Microsoft.Extensions.Logging,Microsoft.Extensions.Options.ConfigurationExtensions,Microsoft.Extensions.Options.DataAnnotations,Microsoft.Extensions.Options,Microsoft.Extensions.Primitives,System.Collections.Immutable,System.Configuration.ConfigurationManager,System.Diagnostics.DiagnosticSource,System.Diagnostics.PerformanceCounter,System.IO.Hashing,System.Net.Http.Json,System.Security.Cryptography.Pkcs,System.Security.Cryptography.Xml,System.Text.Encodings.Web,System.Text.Json,System.Runtime.Caching From Version 8.0.0-preview.6.23314.15 -> To Version 8.0.0-preview.6.23323.4 (parent: Microsoft.AspNetCore.App.Runtime.win-x64 * Update dependencies from https://github.com/dotnet/aspnetcore build 20230625.3 Microsoft.AspNetCore.App.Runtime.win-x64 , Microsoft.AspNetCore.Mvc.Testing , Microsoft.AspNetCore.TestHost , Microsoft.Extensions.Caching.StackExchangeRedis , Microsoft.Extensions.Diagnostics.HealthChecks , Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions , Microsoft.Extensions.Features , Microsoft.Extensions.Http.Polly , Microsoft.Extensions.ObjectPool From Version 8.0.0-preview.6.23315.13 -> To Version 8.0.0-preview.6.23325.3 Dependency coherency updates Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Abstractions,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Configuration.Binder,Microsoft.Extensions.Configuration.CommandLine,Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Configuration,Microsoft.Extensions.DependencyInjection.Abstractions,Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Hosting.Abstractions,Microsoft.Extensions.Hosting,Microsoft.Extensions.Http,Microsoft.Extensions.Logging.Abstractions,Microsoft.Extensions.Logging.Configuration,Microsoft.Extensions.Logging.Console,Microsoft.Extensions.Logging,Microsoft.Extensions.Options.ConfigurationExtensions,Microsoft.Extensions.Options.DataAnnotations,Microsoft.Extensions.Options,Microsoft.Extensions.Primitives,System.Collections.Immutable,System.Configuration.ConfigurationManager,System.Diagnostics.DiagnosticSource,System.Diagnostics.PerformanceCounter,System.IO.Hashing,System.Net.Http.Json,System.Security.Cryptography.Pkcs,System.Security.Cryptography.Xml,System.Text.Encodings.Web,System.Text.Json,System.Runtime.Caching From Version 8.0.0-preview.6.23314.15 -> To Version 8.0.0-preview.6.23323.4 (parent: Microsoft.AspNetCore.App.Runtime.win-x64 * Deprecate this repo's option validator code gen in favor of the runtime'src - This removes traces of the option code generator in this repo, now that the code has moved to dotnet/runtime. Unfortunately, the generator in dotnet/runtime has currently dropped the use of the 'file' visibility accessor in generated code, which leads to some conflicting type definitions in this code base due to the use of IntervalsVisibleTo. This surfaces as build warnings which are safe to ignore, since the C# compiler ends up binding to the right thing. Fixing these warnings will require a new drop of the runtime's generator. * Bump SDK * Suppress CS0436 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Igor Velikorossov <[email protected]> Co-authored-by: Martin Taillefer <[email protected]>

MihaZupan added 3 commits June 14, 2023 15:39

Add ContainsAny{Except}

7b56e30

Use it everywhere

86ff910

Test coverage

51b47b5

MihaZupan added the area-System.Memory label Jun 15, 2023

MihaZupan added this to the 8.0.0 milestone Jun 15, 2023

MihaZupan self-assigned this Jun 15, 2023

dotnet-issue-labeler bot added the new-api-needs-documentation label Jun 15, 2023

stephentoub reviewed Jun 15, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs Outdated Show resolved Hide resolved

stephentoub approved these changes Jun 15, 2023

View reviewed changes

stephentoub reviewed Jun 15, 2023

View reviewed changes

src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 15, 2023

View reviewed changes

src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs Outdated Show resolved Hide resolved

MihaZupan added 5 commits June 15, 2023 18:07

Fix ContainsAny on a string

1db908e

Reword summaries on ContainsAny

1009481

Appease older TFMs

4e5da31

Revert TestFilter.cs changes

909a331

Appease older TFMs part 2

e7651e8

This was referenced Jun 15, 2023

Tracking issue for CI build timeouts #76454

Closed

Could not load file or assembly 'Microsoft.CodeAnalysis.NetAnalyzers #84995

Closed

Unable to load Analyzer assembly .../Microsoft.CodeAnalysis.Analyzers.dll : Not a valid assembly #85082

Closed

stephentoub approved these changes Jun 16, 2023

View reviewed changes

stephentoub merged commit cb18a1a into dotnet:main Jun 16, 2023

RussKie mentioned this pull request Jun 19, 2023

[main] Update dependencies from dotnet/aspnetcore dotnet/extensions#4086

Closed

RussKie added a commit to dotnet/extensions that referenced this pull request Jun 19, 2023

Bump the SDK to resolve the break caused by dotnet/runtime#87621

fe0feb2

RussKie added a commit to dotnet/extensions that referenced this pull request Jun 21, 2023

Bump the SDK to resolve the break caused by dotnet/runtime#87621

2b0715d

RussKie added a commit to dotnet/extensions that referenced this pull request Jun 23, 2023

Bump the SDK to resolve the break caused by dotnet/runtime#87621

0bd383e

RussKie added a commit to dotnet/extensions that referenced this pull request Jun 26, 2023

Bump the SDK to resolve the break caused by dotnet/runtime#87621

3f0b1fb

RussKie added a commit to dotnet/extensions that referenced this pull request Jun 26, 2023

fixup! Bump the SDK to resolve the break caused by dotnet/runtime#87621

4f785c6

ghost locked as resolved and limited conversation to collaborators Jul 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ContainsAny{Except} #87621

Add ContainsAny{Except} #87621

MihaZupan commented Jun 15, 2023

dotnet-issue-labeler bot commented Jun 15, 2023

ghost commented Jun 15, 2023

stephentoub Jun 15, 2023

MihaZupan Jun 15, 2023

stephentoub Jun 16, 2023

EgorBo Jun 16, 2023

stephentoub Jun 16, 2023

EgorBo Jun 16, 2023

EgorBo Jun 16, 2023

stephentoub left a comment

Add ContainsAny{Except} #87621

Add ContainsAny{Except} #87621

Conversation

MihaZupan commented Jun 15, 2023

dotnet-issue-labeler bot commented Jun 15, 2023

ghost commented Jun 15, 2023

stephentoub Jun 15, 2023

Choose a reason for hiding this comment

MihaZupan Jun 15, 2023

Choose a reason for hiding this comment

stephentoub Jun 16, 2023

Choose a reason for hiding this comment

EgorBo Jun 16, 2023

Choose a reason for hiding this comment

stephentoub Jun 16, 2023

Choose a reason for hiding this comment

EgorBo Jun 16, 2023

Choose a reason for hiding this comment

EgorBo Jun 16, 2023

Choose a reason for hiding this comment

stephentoub left a comment

Choose a reason for hiding this comment