Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using interceptors for IndexOfAny-like calls #91743

Open
MihaZupan opened this issue Sep 7, 2023 · 3 comments
Open

Consider using interceptors for IndexOfAny-like calls #91743

MihaZupan opened this issue Sep 7, 2023 · 3 comments
Labels
area-System.Memory needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@MihaZupan
Copy link
Member

MihaZupan commented Sep 7, 2023

dotnet/roslyn-analyzers#6898 is adding an analyzer that will flag uses like

const string MyValues = "abcdef";

span.IndexOfAny(MyValues);

and offer a fixer to replace it with

private static readonly SearchValues<char> s_myValues = SearchValues.Create(MyValues);
const string MyValues = "abcdef";

span.IndexOfAny(s_myValues);

For patterns like these, we should consider using interceptors to automagically rewrite them to use SearchValues, without requiring the user to extract the values to a field.
The logic could also be smarter and rewrite the above example to use IndexOfAnyInRange directly instead.

This would also help projects that multi-target to TFMs that don't have SearchValues, as it would let them take advantage of the perf improvements without extra #if #elses.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 7, 2023
@ghost
Copy link

ghost commented Sep 7, 2023

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

Issue Details

dotnet/roslyn-analyzers#6898 is adding an analyzer that will flag uses like

const string MyValues = "abcdef";

span.IndexOfAny(MyValues);

and offer a fixer to replace it with

private static readonly SearchValues<char> s_myValues = SearchValues.Create(MyValues);
const string MyValues = "abcdef";

span.IndexOfAny(s_myValues);

For patterns like these, we should consider using interceptors to automagically rewrite them to use SearchValues, without requiring the user to extract the values to a field.

This would also help project that multi-target to TFMs that don't have SearchValues, as it would let them take advantage of the perf improvements without extra #if #elses.

Author: MihaZupan
Assignees: -
Labels:

area-System.Memory

Milestone: -

@adamsitnik
Copy link
Member

It sounds reasonable, but does it have any downsides? Like longer startup (initialization of a static field) etc? If so, how could we tell whether it should be applied to all possible places or only selected ones?

@MihaZupan
Copy link
Member Author

Yes, it could affect startup somewhat. Creating the SearchValues themselves is pretty cheap (order of 100s of ns), so my guess is that any runtime costs of having extra fields / cctors could show up.
I wouldn't expect users to have too many such instances in their apps though.

@tannergooding tannergooding added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Jun 24, 2024
@stephentoub stephentoub added this to the Future milestone Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Memory needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

4 participants