Performance improvements in SymbolNamesWithValueOption<T>

[Alex-Sob]

I would like to suggest some performance improvements in SymbolNamesWithValueOption<TValue> class:

• Currently the nested SymbolNamesWithValueOption<TValue>.NameParts type is a class, it seems that it could be a struct instead as it it's a type that just holds two values.

• Create method counts wildcard characters by calling Count method on a string. It can be replaced with IndexOf which removes a delegate (and maybe could even take advantage of vectorization, if runtime supports that). Also, the check if there's more than one wildcard can be removed because if a symbol name contains a wildcard then the method expects that it's the last character, otherwise it skips processing it.

• Fixed CA1847 in Create method and removed suppression.

[KalleOlaviNiemitalo]

In System.CommandLine, there was some earlier pull request that changed value types to reference types, to speed up application startup. IIRC, the effect was based on letting the JIT compiler use the same generated code for those types as for other reference types, when they are used as type arguments.

I don't know whether SymbolNamesWithValueOption<TValue> is used in a way that makes the startup latency depend on whether it's a value type or a reference type. And if the .NET SDK becomes native AOT compiled, then that may affect which one is faster. Anyway, if there is a performance difference here, I think it should be backed by measurements rather than expectations.

[baronfel]

@KalleOlaviNiemitalo just a nit: if the SDK becomes AOT that says nothing about how compilation would occur - MSbuild builds will continue to happen in a CoreCLR bubble, the Roslyn compiler will continue to be CoreCLR-hosted, and Analyzers only run in the context of a Roslyn instance. Even for file-based apps compilation would continue to be CoreCLR-based for quite some time.

[jaredpar]

I would like to suggest some performance improvements in SymbolNamesWithValueOption class:

What measuring did you do to validate this is actually improving performance?

[Alex-Sob]

What measuring did you do to validate this is actually improving performance?

@jaredpar I'll see if I can benchmark tests or something

[github-actions]

Due to lack of recent activity, this PR has been labeled as 'Stale'. It will be closed if no further activity occurs within 7 more days. Any new comment will remove the label.