Avoid unnecessary argument-builder work in generic type substitution

src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj

@@ -84,6 +84,7 @@
     <InternalsVisibleTo Include="Microsoft.CodeAnalysis.CSharp.Features.UnitTests"/>
     <InternalsVisibleTo Include="Microsoft.CodeAnalysis.EditorFeatures2.UnitTests" />
     <InternalsVisibleTo Include="CompilerBenchmarks" />
+    <InternalsVisibleTo Include="Benchmarks" />
     <InternalsVisibleTo Include="Microsoft.Build.Tasks.CodeAnalysis.UnitTests" />
     <InternalsVisibleTo Include="Microsoft.CodeAnalysis.Features.Test.Utilities" />
     <InternalsVisibleTo Include="Microsoft.CodeAnalysis.LanguageServer.Protocol.Test.Utilities" />

src/Compilers/CSharp/Portable/Symbols/AbstractTypeMap.cs

@@ -58,29 +58,34 @@ internal NamedTypeSymbol SubstituteNamedType(NamedTypeSymbol previous)
             NamedTypeSymbol newConstructedFrom = SubstituteTypeDeclaration(oldConstructedFrom);
 
             ImmutableArray<TypeWithAnnotations> oldTypeArguments = previous.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics;
-            bool changed = !ReferenceEquals(oldConstructedFrom, newConstructedFrom);
-            var newTypeArguments = ArrayBuilder<TypeWithAnnotations>.GetInstance(oldTypeArguments.Length);
+            ArrayBuilder<TypeWithAnnotations> newTypeArguments = null;
 
             for (int i = 0; i < oldTypeArguments.Length; i++)
             {
                 var oldArgument = oldTypeArguments[i];
                 var newArgument = oldArgument.SubstituteType(this);
 
-                if (!changed && !oldArgument.IsSameAs(newArgument))
+                if (newTypeArguments is null)
                 {
-                    changed = true;
+                    if (oldArgument.IsSameAs(newArgument))
+                    {
+                        continue;
+                    }
+
+                    newTypeArguments = ArrayBuilder<TypeWithAnnotations>.GetInstance(oldTypeArguments.Length);
+                    newTypeArguments.AddRange(oldTypeArguments, i);
                 }
 
                 newTypeArguments.Add(newArgument);
             }
 
-            if (!changed)
+            if (newTypeArguments is null && ReferenceEquals(oldConstructedFrom, newConstructedFrom))
             {
-                newTypeArguments.Free();
                 return previous;
             }
 
-            return newConstructedFrom.ConstructIfGeneric(newTypeArguments.ToImmutableAndFree()).WithTupleDataFrom(previous);
+            var substitutedArguments = newTypeArguments is null ? oldTypeArguments : newTypeArguments.ToImmutableAndFree();
+            return newConstructedFrom.ConstructIfGeneric(substitutedArguments).WithTupleDataFrom(previous);
         }
 
         /// <summary>

src/Compilers/CSharp/Test/Symbol/Symbols/Source/TypeMapTests.cs

@@ -11,13 +11,201 @@
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
 using Microsoft.CodeAnalysis.PooledObjects;
+using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.CodeAnalysis.Text;
+using Roslyn.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.CSharp.UnitTests
 {
     public class TypeMapTests : CSharpTestBase
     {
+        [Theory]
+        [InlineData("C")]
+        [InlineData("C<string>")]
+        [InlineData("C<string, int>")]
+        [InlineData("C<string, int, byte, char, long, short, bool, object>")]
+        public void SubstituteNamedType_NoChange(string type)
+        {
+            var (map, previous, _) = CreateSubstitution(type, type);
+
+            Assert.Same(previous, map.SubstituteNamedType(previous));
+        }
+
+        [Theory]
+        [InlineData("C<T>", "C<int>")]
+        [InlineData("C<T, string>", "C<int, string>")]
+        [InlineData("C<string, T>", "C<string, int>")]
+        [InlineData("C<T, string, byte, char, long, short, bool, object>", "C<int, string, byte, char, long, short, bool, object>")]
+        [InlineData("C<string, byte, char, long, short, bool, object, T>", "C<string, byte, char, long, short, bool, object, int>")]
+        [InlineData("C<string, C<T, T>>", "C<string, C<int, int>>")]
+        [InlineData("Outer<T>.C<C<string, T>>", "Outer<int>.C<C<string, int>>")]
+        public void SubstituteNamedType_ChangedArguments(string type, string substitutedType)
+        {
+            var (map, previous, expected) = CreateSubstitution(type, substitutedType);
+
+            var actual = map.SubstituteNamedType(previous);
+
+            Assert.NotSame(previous, actual);
+            Assert.True(TypeSymbol.Equals(expected, actual, TypeCompareKind.ConsiderEverything));
+            Assert.Same(previous.OriginalDefinition, actual.OriginalDefinition);
+            Assert.True(TypeSymbol.Equals(actual, map.SubstituteNamedType(actual), TypeCompareKind.ConsiderEverything));
+        }
+
+        [Theory]
+        [InlineData("Outer<T>.C", "Outer<int>.C")]
+        [InlineData("Outer<T>.C<string>", "Outer<int>.C<string>")]
+        [InlineData("Outer<T>.C<string, byte>", "Outer<int>.C<string, byte>")]
+        [InlineData("Outer<T>.C<string, byte, char, long, short, bool, object, double>", "Outer<int>.C<string, byte, char, long, short, bool, object, double>")]
+        public void SubstituteNamedType_ContainingTypeOnly(string type, string substitutedType)
+        {
+            var (map, previous, expected) = CreateSubstitution(type, substitutedType);
+
+            var actual = map.SubstituteNamedType(previous);
+
+            Assert.NotSame(previous, actual);
+            Assert.True(TypeSymbol.Equals(expected, actual, TypeCompareKind.ConsiderEverything));
+            Assert.Same(previous.OriginalDefinition, actual.OriginalDefinition);
+            Assert.Equal(SpecialType.System_Int32, actual.ContainingType.TypeArguments().Single().SpecialType);
+            var oldArguments = previous.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics;
+            var newArguments = actual.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics;
+            Assert.Equal(oldArguments.Length, newArguments.Length);
+            for (int i = 0; i < oldArguments.Length; i++)
+            {
+                Assert.True(oldArguments[i].Equals(newArguments[i], TypeCompareKind.ConsiderEverything));
+            }
+        }
+
+        private static (TypeMap map, NamedTypeSymbol previous, NamedTypeSymbol expected) CreateSubstitution(string type, string substitutedType)
+        {
+            var compilation = CreateCompilation($$"""
+                public class C { }
+                public class C<T> { }
+                public class C<T1, T2> { }
+                public class C<T1, T2, T3, T4, T5, T6, T7, T8> { }
+                public class Outer<T>
+                {
+                    public class C { }
+                    public class C<T1> { }
+                    public class C<T1, T2> { }
+                    public class C<T1, T2, T3, T4, T5, T6, T7, T8> { }
+                }
+                public class Context<T>
+                {
+                    public {{type}} Previous { get; set; }
+                    public {{substitutedType}} Expected { get; set; }
+                }
+                """);
+            compilation.VerifyEmitDiagnostics();
+            var context = compilation.GetTypeByMetadataName("Context`1");
+            var map = new TypeMap(context.TypeParameters,
+                ImmutableArray.Create(TypeWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Int32))));
+            var previous = (NamedTypeSymbol)((PropertySymbol)context.GetMembers("Previous").Single()).Type;
+            var expected = (NamedTypeSymbol)((PropertySymbol)context.GetMembers("Expected").Single()).Type;
+            return (map, previous, expected);
+        }
+
+        [Fact]
+        public void SubstituteNamedType_TupleNamesAndNullableArguments()
+        {
+            var compilation = CreateCompilation("""
+                #nullable enable
+                public class C<T1, T2> { }
+                public class Context<T> where T : class
+                {
+                    public C<string?, (T? first, C<string?, T> second)> Previous => throw null!;
+                    public C<string?, (object? first, C<string?, object> second)> Expected => throw null!;
+                }
+                """, targetFramework: TargetFramework.NetCoreApp);
+            compilation.VerifyEmitDiagnostics();
+            var context = compilation.GetTypeByMetadataName("Context`1");
+            var previous = (NamedTypeSymbol)((PropertySymbol)context.GetMembers("Previous").Single()).Type;
+            var expected = (NamedTypeSymbol)((PropertySymbol)context.GetMembers("Expected").Single()).Type;
+            var map = new TypeMap(context.TypeParameters,
+                ImmutableArray.Create(TypeWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Object), NullableAnnotation.NotAnnotated)));
+
+            var actual = map.SubstituteNamedType(previous);
+
+            Assert.True(TypeSymbol.Equals(expected, actual, TypeCompareKind.ConsiderEverything));
+            var arguments = actual.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics;
+            Assert.Equal(NullableAnnotation.Annotated, arguments[0].NullableAnnotation);
+            var tuple = (NamedTypeSymbol)arguments[1].Type;
+            Assert.Equal(new[] { "first", "second" }, tuple.TupleElementNames);
+            Assert.Equal(NullableAnnotation.Annotated, tuple.TupleElements[0].TypeWithAnnotations.NullableAnnotation);
+            Assert.Equal(SpecialType.System_Object, tuple.TupleElements[0].Type.SpecialType);
+        }
+
+        [Fact]
+        public void SubstituteNamedType_NullabilityOnly()
+        {
+            var compilation = CreateCompilation("""
+                #nullable enable
+                public class C<T1, T2> { }
+                public class Context<T> where T : class
+                {
+                    public C<string?, T> Previous => throw null!;
+                    public C<string?, T?> Expected => throw null!;
+                }
+                """);
+            compilation.VerifyEmitDiagnostics();
+            var context = compilation.GetTypeByMetadataName("Context`1");
+            var previous = (NamedTypeSymbol)((PropertySymbol)context.GetMembers("Previous").Single()).Type;
+            var expected = (NamedTypeSymbol)((PropertySymbol)context.GetMembers("Expected").Single()).Type;
+            var map = new TypeMap(context.TypeParameters,
+                ImmutableArray.Create(TypeWithAnnotations.Create(context.TypeParameters.Single(), NullableAnnotation.Annotated)));
+
+            var actual = map.SubstituteNamedType(previous);
+
+            Assert.NotSame(previous, actual);
+            Assert.True(TypeSymbol.Equals(expected, actual, TypeCompareKind.ConsiderEverything));
+            var oldArgument = previous.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[1];
+            var newArgument = actual.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[1];
+            Assert.Same(oldArgument.Type, newArgument.Type);
+            Assert.Equal(NullableAnnotation.NotAnnotated, oldArgument.NullableAnnotation);
+            Assert.Equal(NullableAnnotation.Annotated, newArgument.NullableAnnotation);
+            Assert.False(oldArgument.IsSameAs(newArgument));
+        }
+
+        [Fact]
+        public void SubstituteNamedType_CustomModifierOnly()
+        {
+            var compilation = CreateCompilation("""
+                public class C<T1, T2> { }
+                public class Modifier<T> { }
+                """);
+            compilation.VerifyEmitDiagnostics();
+            var definition = compilation.GetTypeByMetadataName("C`2");
+            var modifier = compilation.GetTypeByMetadataName("Modifier`1");
+            var intType = compilation.GetSpecialType(SpecialType.System_Int32);
+            var stringType = compilation.GetSpecialType(SpecialType.System_String);
+            var previous = definition.Construct(ImmutableArray.Create(
+                TypeWithAnnotations.Create(stringType, NullableAnnotation.Annotated),
+                TypeWithAnnotations.Create(intType, customModifiers: ImmutableArray.Create<CustomModifier>(
+                    CSharpCustomModifier.CreateOptional(modifier),
+                    CSharpCustomModifier.CreateRequired(stringType)))));
+            var map = new TypeMap(modifier.TypeParameters, ImmutableArray.Create(TypeWithAnnotations.Create(intType)));
+
+            var actual = map.SubstituteNamedType(previous);
+
+            Assert.NotSame(previous, actual);
+            var arguments = actual.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics;
+            Assert.Same(stringType, arguments[0].Type);
+            Assert.Equal(NullableAnnotation.Annotated, arguments[0].NullableAnnotation);
+            Assert.Same(intType, arguments[1].Type);
+            Assert.Collection(arguments[1].CustomModifiers,
+                m =>
+                {
+                    Assert.True(m.IsOptional);
+                    Assert.True(TypeSymbol.Equals(modifier.Construct(intType), ((CSharpCustomModifier)m).ModifierSymbol, TypeCompareKind.ConsiderEverything));
+                },
+                m =>
+                {
+                    Assert.False(m.IsOptional);
+                    Assert.Same(stringType, ((CSharpCustomModifier)m).ModifierSymbol);
+                });
+            Assert.False(previous.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[1].IsSameAs(arguments[1]));
+        }
+
         // take a type of the form Something<X> and return the type X.
         private TypeSymbol TypeArg(TypeSymbol t)
         {

src/Compilers/Core/Portable/Microsoft.CodeAnalysis.csproj

@@ -102,6 +102,7 @@
     <InternalsVisibleTo Include="VBCSCompiler.UnitTests" />
     <InternalsVisibleTo Include="DynamicProxyGenAssembly2" Key="$(MoqPublicKey)" LoadsWithinVisualStudio="false" />
     <InternalsVisibleTo Include="CompilerBenchmarks" />
+    <InternalsVisibleTo Include="Benchmarks" />
 
     <EmbeddedResource Update="CodeAnalysisResources.resx" GenerateSource="true" />
     <EmbeddedResource Include="Resources\default.win32manifest" />

src/Tools/Benchmarks/GenericTypeSubstitutionBenchmarks.cs

@@ -0,0 +1,120 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Immutable;
+using System.Linq;
+using Basic.Reference.Assemblies;
+using BenchmarkDotNet.Attributes;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.Text;
+
+namespace Benchmarks;
+
+public enum GenericTypeSubstitutionCase
+{
+    NoChange0,
+    NoChange1,
+    NoChange2,
+    NoChange8,
+    FirstChanged1,
+    FirstChanged2,
+    FirstChanged8,
+    LastChanged2,
+    LastChanged8,
+    ContainingTypeChanged0,
+    ContainingTypeChanged1,
+    ContainingTypeChanged2,
+    ContainingTypeChanged8,
+    NestedArgumentChanged,
+}
+
+[MemoryDiagnoser]
+public class GenericTypeSubstitutionBenchmarks
+{
+    private TypeMap _map = null!;
+    private NamedTypeSymbol _type = null!;
+
+    [ParamsAllValues]
+    public GenericTypeSubstitutionCase Case { get; set; }
+
+    [GlobalSetup]
+    public void Setup()
+    {
+        var arity = Case switch
+        {
+            GenericTypeSubstitutionCase.NoChange0 or GenericTypeSubstitutionCase.ContainingTypeChanged0 => 0,
+            GenericTypeSubstitutionCase.NoChange1 or GenericTypeSubstitutionCase.FirstChanged1 or
+                GenericTypeSubstitutionCase.ContainingTypeChanged1 => 1,
+            GenericTypeSubstitutionCase.NoChange2 or GenericTypeSubstitutionCase.FirstChanged2 or
+                GenericTypeSubstitutionCase.LastChanged2 or GenericTypeSubstitutionCase.ContainingTypeChanged2 or
+                GenericTypeSubstitutionCase.NestedArgumentChanged => 2,
+            GenericTypeSubstitutionCase.NoChange8 or GenericTypeSubstitutionCase.FirstChanged8 or
+                GenericTypeSubstitutionCase.LastChanged8 or GenericTypeSubstitutionCase.ContainingTypeChanged8 => 8,
+            _ => throw new InvalidOperationException(),
+        };
+        var typeParameters = arity == 0
+            ? ""
+            : "<" + string.Join(", ", Enumerable.Range(0, arity).Select(i => $"T{i}")) + ">";
+        var source = $$"""
+            public class Parameters<T> { }
+            public class G{{typeParameters}} { }
+            public class Outer<T>
+            {
+                public class Inner{{typeParameters}} { }
+            }
+            """;
+        var compilation = CSharpCompilation.Create(
+            nameof(GenericTypeSubstitutionBenchmarks),
+            [CSharpSyntaxTree.ParseText(SourceText.From(source))],
+            Net90.References.All,
+            new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
+        var errors = compilation.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error).ToArray();
+        if (errors.Length != 0)
+        {
+            throw new InvalidOperationException(string.Join(Environment.NewLine, errors.Select(d => d.ToString())));
+        }
+
+        var parameter = compilation.GlobalNamespace.GetTypeMembers("Parameters").Single().TypeParameters.Single();
+        var intType = compilation.GetSpecialType(SpecialType.System_Int32);
+        var stringType = compilation.GetSpecialType(SpecialType.System_String);
+        _map = new TypeMap(
+            ImmutableArray.Create(parameter),
+            ImmutableArray.Create(TypeWithAnnotations.Create(intType)));
+
+        var definition = compilation.GlobalNamespace.GetTypeMembers("G").Single();
+        if (Case is GenericTypeSubstitutionCase.ContainingTypeChanged0 or
+            GenericTypeSubstitutionCase.ContainingTypeChanged1 or
+            GenericTypeSubstitutionCase.ContainingTypeChanged2 or
+            GenericTypeSubstitutionCase.ContainingTypeChanged8)
+        {
+            definition = compilation.GlobalNamespace.GetTypeMembers("Outer").Single()
+                .Construct(parameter).GetTypeMembers("Inner").Single();
+        }
+
+        var arguments = Enumerable.Repeat<TypeSymbol>(stringType, arity).ToArray();
+        switch (Case)
+        {
+            case GenericTypeSubstitutionCase.FirstChanged1:
+            case GenericTypeSubstitutionCase.FirstChanged2:
+            case GenericTypeSubstitutionCase.FirstChanged8:
+                arguments[0] = parameter;
+                break;
+            case GenericTypeSubstitutionCase.LastChanged2:
+            case GenericTypeSubstitutionCase.LastChanged8:
+                arguments[arity - 1] = parameter;
+                break;
+            case GenericTypeSubstitutionCase.NestedArgumentChanged:
+                arguments[0] = definition.Construct(parameter, stringType);
+                break;
+        }
+
+        _type = arity == 0 ? definition : definition.Construct(arguments);
+    }
+
+    [Benchmark]
+    public object Substitute() => _map.SubstituteNamedType(_type);
+}