Conversation
| int n = array.Length; | ||
| for (int i = 0; i < n; i++) | ||
| { | ||
| var a = array[i]; |
There was a problem hiding this comment.
Why not basic for each? It's non allocating for immutable array right?
There was a problem hiding this comment.
I'm not sure why everything in this file is written this way, but I prefer not to change it for just one case without cause, especially as part of a performance PR.
| { | ||
| if ((namespace1Count > 1 && await namespace1.ConstituentNamespaces.AnyAsync(n => NamespaceSymbolsMatchAsync(solution, n, namespace2, cancellationToken)).ConfigureAwait(false)) || | ||
| (namespace2Count > 1 && await namespace2.ConstituentNamespaces.AnyAsync(n2 => NamespaceSymbolsMatchAsync(solution, namespace1, n2, cancellationToken)).ConfigureAwait(false))) | ||
| if ((namespace1Count > 1 && await namespace1.ConstituentNamespaces.AnyAsync(static (n, arg) => NamespaceSymbolsMatchAsync(arg.solution, n, arg.namespace2, arg.cancellationToken), (solution, namespace2, cancellationToken)).ConfigureAwait(false)) || |
There was a problem hiding this comment.
Can the other extension method be deleted in favor of the new one?
There was a problem hiding this comment.
I'm not sure. The other one is perfectly valid so I didn't look outside the ones that appeared in the trace.
There was a problem hiding this comment.
I gave a look and it seems this PR replaced all usages of the old extension method.
https://grep.app/search?q=.AnyAsync&filter[repo][0]=dotnet/roslyn
It had only 3 usages which you replaced in the PR. I think it should be fine either way.
These captures showed up as a few GB during a long-running high CPU operation measured locally.