From d4feb3cb70e6715e9eade75e4a6e444fd2cef03d Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Tue, 6 Dec 2022 00:30:52 -0800 Subject: [PATCH 1/3] Fix quadratic algorithm in CompilerGeneratedState The way this code was supposed to work was that it would scan the compiler- generated type and all its descendents, record each generated type it found, then fill in information for all of the found types. The way it actually worked was that it would scan the descendents, record each generated type, then try to fill in information *for all generated types found in the program*. This is quadratic as you start adding types, as you rescan everything you've added before. The fix is to record just the types from the current pass, and then add them to the larger bag when everything's complete. --- .../Linker.Dataflow/CompilerGeneratedState.cs | 74 ++++++++++++++----- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs index 36950fb9c03f..8260bf515e37 100644 --- a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs +++ b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs @@ -126,6 +126,7 @@ public static bool TryGetStateMachineType (MethodDefinition method, [NotNullWhen var callGraph = new CompilerGeneratedCallGraph (); var userDefinedMethods = new HashSet (); + var generatedTypeToTypeArgs = new Dictionary (); void ProcessMethod (MethodDefinition method) { @@ -152,14 +153,15 @@ void ProcessMethod (MethodDefinition method) if (referencedMethod == null) continue; + // Find calls to state machine constructors that occur outside the type if (referencedMethod.IsConstructor && referencedMethod.DeclaringType is var generatedType && // Don't consider calls in the same type, like inside a static constructor method.DeclaringType != generatedType && CompilerGeneratedNames.IsLambdaDisplayClass (generatedType.Name)) { // fill in null for now, attribute providers will be filled in later - if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, new TypeArgumentInfo (method, null))) { - var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod; + if (!generatedTypeToTypeArgs.TryAdd (generatedType, new TypeArgumentInfo (method, null))) { + var alreadyAssociatedMethod = generatedTypeToTypeArgs[generatedType].CreatingMethod; _context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ()); } continue; @@ -189,7 +191,7 @@ referencedMethod.DeclaringType is var generatedType && // Don't consider field accesses in the same type, like inside a static constructor method.DeclaringType != generatedType && CompilerGeneratedNames.IsLambdaDisplayClass (generatedType.Name)) { - if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, new TypeArgumentInfo (method, null))) { + if (!generatedTypeToTypeArgs.TryAdd (generatedType, new TypeArgumentInfo (method, null))) { // It's expected that there may be multiple methods associated with the same static closure environment. // All of these methods will substitute the same type arguments into the closure environment // (if it is generic). Don't warn. @@ -214,7 +216,7 @@ referencedMethod.DeclaringType is var generatedType && } // Already warned above if multiple methods map to the same type // Fill in null for argument providers now, the real providers will be filled in later - _generatedTypeToTypeArgumentInfo[stateMachineType] = new TypeArgumentInfo (method, null); + generatedTypeToTypeArgs[stateMachineType] = new TypeArgumentInfo (method, null); } } @@ -280,9 +282,18 @@ referencedMethod.DeclaringType is var generatedType && // Now that we have instantiating methods fully filled out, walk the generated types and fill in the attribute // providers - foreach (var generatedType in _generatedTypeToTypeArgumentInfo.Keys) { + foreach (var generatedType in generatedTypeToTypeArgs.Keys) { if (HasGenericParameters (generatedType)) - MapGeneratedTypeTypeParameters (generatedType); + { + MapGeneratedTypeTypeParameters (generatedType, generatedTypeToTypeArgs, _context); + // Finally, add resolved type arguments to the cache + var info = generatedTypeToTypeArgs[generatedType]; + if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, info)) { + var method = info.CreatingMethod; + var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod; + _context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ()); + } + } } _cachedTypeToCompilerGeneratedMembers.Add (type, compilerGeneratedCallees); @@ -301,18 +312,42 @@ static bool HasGenericParameters (TypeDefinition typeDef) return typeDef.GenericParameters.Count > typeDef.DeclaringType.GenericParameters.Count; } - void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) + /// + /// Attempts to reverse the process of the compiler's alpha renaming. So if the original code was + /// something like this: + /// + /// void M<T> () { + /// Action a = () => { Console.WriteLine (typeof (T)); }; + /// } + /// + /// The compiler will generate a nested class like this: + /// + /// class <>c__DisplayClass0<T> { + /// public void <M>b__0 () { + /// Console.WriteLine (typeof (T)); + /// } + /// } + /// + /// The task of this method is to figure out that the type parameter T in the nested class is the same + /// as the type parameter T in the parent method M. + /// acts as a memoization table to avoid recalculating the + /// mapping multiple times. + /// + static void MapGeneratedTypeTypeParameters ( + TypeDefinition generatedType, + Dictionary generatedTypeToTypeArgs, + LinkContext context) { Debug.Assert (CompilerGeneratedNames.IsGeneratedType (generatedType.Name)); - var typeInfo = _generatedTypeToTypeArgumentInfo[generatedType]; + var typeInfo = generatedTypeToTypeArgs[generatedType]; if (typeInfo.OriginalAttributes is not null) { return; } var method = typeInfo.CreatingMethod; if (method.Body is { } body) { var typeArgs = new ICustomAttributeProvider[generatedType.GenericParameters.Count]; - var typeRef = ScanForInit (generatedType, body); + var typeRef = ScanForInit (generatedType, body, context); if (typeRef is null) { return; } @@ -334,9 +369,9 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) var owningRef = (TypeReference) owner; if (!CompilerGeneratedNames.IsGeneratedType (owningRef.Name)) { userAttrs = param; - } else if (_context.TryResolve ((TypeReference) param.Owner) is { } owningType) { - MapGeneratedTypeTypeParameters (owningType); - if (_generatedTypeToTypeArgumentInfo[owningType].OriginalAttributes is { } owningAttrs) { + } else if (context.TryResolve ((TypeReference) param.Owner) is { } owningType) { + MapGeneratedTypeTypeParameters (owningType, generatedTypeToTypeArgs, context); + if (generatedTypeToTypeArgs[owningType].OriginalAttributes is { } owningAttrs) { userAttrs = owningAttrs[param.Position]; } else { Debug.Assert (false, "This should be impossible in valid code"); @@ -348,19 +383,22 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) typeArgs[i] = userAttrs; } - _generatedTypeToTypeArgumentInfo[generatedType] = typeInfo with { OriginalAttributes = typeArgs }; + generatedTypeToTypeArgs[generatedType] = typeInfo with { OriginalAttributes = typeArgs }; } } - GenericInstanceType? ScanForInit (TypeDefinition compilerGeneratedType, MethodBody body) + static GenericInstanceType? ScanForInit ( + TypeDefinition compilerGeneratedType, + MethodBody body, + LinkContext context) { - foreach (var instr in _context.GetMethodIL (body).Instructions) { + foreach (var instr in context.GetMethodIL (body).Instructions) { bool handled = false; switch (instr.OpCode.Code) { case Code.Initobj: case Code.Newobj: { if (instr.Operand is MethodReference { DeclaringType: GenericInstanceType typeRef } - && compilerGeneratedType == _context.TryResolve (typeRef)) { + && compilerGeneratedType == context.TryResolve (typeRef)) { return typeRef; } handled = true; @@ -368,7 +406,7 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) break; case Code.Stsfld: { if (instr.Operand is FieldReference { DeclaringType: GenericInstanceType typeRef } - && compilerGeneratedType == _context.TryResolve (typeRef)) { + && compilerGeneratedType == context.TryResolve (typeRef)) { return typeRef; } handled = true; @@ -381,7 +419,7 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) if (!handled && instr.OpCode.OperandType is OperandType.InlineMethod) { if (instr.Operand is GenericInstanceMethod gim) { foreach (var tr in gim.GenericArguments) { - if (tr is GenericInstanceType git && compilerGeneratedType == _context.TryResolve (git)) { + if (tr is GenericInstanceType git && compilerGeneratedType == context.TryResolve (git)) { return git; } } From 9cc80e90b7a6f17f0d74303961c2f1373fb1b7e7 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 7 Dec 2022 16:36:20 -0800 Subject: [PATCH 2/3] Add perf test generator --- ...rfTestGeneratorForCompilerGeneratedCode.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs diff --git a/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs b/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs new file mode 100644 index 000000000000..65e2be68450d --- /dev/null +++ b/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs @@ -0,0 +1,47 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.IO; +using System.Linq; + +/// +/// This class generates a test that can be used to test perf of analyzing +/// compiler-generated code. Run it by copying this file into a console app and +/// calling . A file +/// will be generated in the current directory named GeneratedLinkerTests.cs. +/// Copy this file into another Console app and trim the app to measure the +/// perf. +/// +static class PerfTestGeneratorForCompilerGeneratedCode +{ + const int FuncNumber = 10000; + public static void Run() + { + using var fstream = File.Create ("GeneratedLinkerTests.cs"); + using var writer = new StreamWriter (fstream); + writer.WriteLine ($$""" +class C { + public static async void Main() + { + int x = 0; + {{string.Join (@" + ", Enumerable.Range (0, FuncNumber).Select (i => $"x += await N{i}.M();"))}} + Console.WriteLine(x); + } +} +"""); + for (int i = 0; i < FuncNumber; i++) { + writer.WriteLine ($$""" +public static class N{{i}} +{ + public static async ValueTask M() + { + Func a = () => 1; + await Task.Delay(0); + return a(); + } +} +"""); + } + } +} \ No newline at end of file From 369670f5dce21c9ff069fc4b8d247942dcc1c557 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 7 Dec 2022 16:38:39 -0800 Subject: [PATCH 3/3] Lint --- src/linker/Linker.Dataflow/CompilerGeneratedState.cs | 3 +-- .../Tests/PerfTestGeneratorForCompilerGeneratedCode.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs index 8260bf515e37..cc7d205b9fb9 100644 --- a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs +++ b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs @@ -283,8 +283,7 @@ referencedMethod.DeclaringType is var generatedType && // Now that we have instantiating methods fully filled out, walk the generated types and fill in the attribute // providers foreach (var generatedType in generatedTypeToTypeArgs.Keys) { - if (HasGenericParameters (generatedType)) - { + if (HasGenericParameters (generatedType)) { MapGeneratedTypeTypeParameters (generatedType, generatedTypeToTypeArgs, _context); // Finally, add resolved type arguments to the cache var info = generatedTypeToTypeArgs[generatedType]; diff --git a/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs b/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs index 65e2be68450d..41f1aa0a798b 100644 --- a/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs +++ b/test/Mono.Linker.Tests/Tests/PerfTestGeneratorForCompilerGeneratedCode.cs @@ -15,7 +15,7 @@ static class PerfTestGeneratorForCompilerGeneratedCode { const int FuncNumber = 10000; - public static void Run() + public static void Run () { using var fstream = File.Create ("GeneratedLinkerTests.cs"); using var writer = new StreamWriter (fstream);