From 89ac45dfd0ab0949da068a6f806dc18c02aca4ee Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Sat, 2 Mar 2024 15:08:38 -0800 Subject: [PATCH 1/7] Reduce closure and Lazy allocations inside CSharpCompilation.ctor These account for about 0.3% of all allocations in the CopyPlainText speedometer test. --- .../Portable/Compilation/CSharpCompilation.cs | 67 ++++++++++--------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 3bac2a10db5aa..0cf6d3743e772 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -50,11 +50,13 @@ public sealed partial class CSharpCompilation : Compilation // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! private readonly CSharpCompilationOptions _options; - private readonly Lazy _usingsFromOptions; - private readonly Lazy> _globalImports; - private readonly Lazy _previousSubmissionImports; - private readonly Lazy _globalNamespaceAlias; // alias symbol used to resolve "global::". - private readonly Lazy _scriptClass; + private UsingsFromOptionsAndDiagnostics? _usingsFromOptions; + private ImmutableArray _globalImports; + private Imports? _previousSubmissionImports; + private AliasSymbol? _globalNamespaceAlias; // alias symbol used to resolve "global::". + + private bool _scriptClassInitialized; + private ImplicitNamedTypeSymbol? _scriptClass; // The type of host object model if available. private TypeSymbol? _lazyHostObjectTypeSymbol; @@ -77,17 +79,7 @@ public sealed partial class CSharpCompilation : Compilation /// A conversions object that ignores nullability. /// internal Conversions Conversions - { - get - { - if (_conversions == null) - { - Interlocked.CompareExchange(ref _conversions, new BuckStopsHereBinder(this, associatedFileIdentifier: null).Conversions, null); - } - - return _conversions; - } - } + => InterlockedOperations.Initialize(ref _conversions, static self => new BuckStopsHereBinder(self, associatedFileIdentifier: null).Conversions, this); /// /// Manages anonymous types declared in this compilation. Unifies types that are structurally equivalent. @@ -473,11 +465,6 @@ private CSharpCompilation( _options = options; this.builtInOperators = new BuiltInOperators(this); - _scriptClass = new Lazy(BindScriptClass); - _globalImports = new Lazy>(BindGlobalImports); - _usingsFromOptions = new Lazy(BindUsingsFromOptions); - _previousSubmissionImports = new Lazy(ExpandPreviousSubmissionImports); - _globalNamespaceAlias = new Lazy(CreateGlobalNamespaceAlias); _anonymousTypeManager = new AnonymousTypeManager(this); this.LanguageVersion = CommonLanguageVersion(syntaxAndDeclarations.ExternalSyntaxTrees); @@ -1503,7 +1490,17 @@ internal bool GetExternAliasTarget(string aliasName, out NamespaceSymbol @namesp /// internal new NamedTypeSymbol? ScriptClass { - get { return _scriptClass.Value; } + get + { + // _scriptClass is allowed to be null, thus why a bool is used to track initialization + if (!_scriptClassInitialized) + { + _scriptClassInitialized = true; + Interlocked.CompareExchange(ref _scriptClass, BindScriptClass(), null); + } + + return _scriptClass; + } } /// @@ -1526,7 +1523,18 @@ internal bool IsSubmissionSyntaxTree(SyntaxTree tree) /// /// Global imports (including those from previous submissions, if there are any). /// - internal ImmutableArray GlobalImports => _globalImports.Value; + internal ImmutableArray GlobalImports + { + get + { + if (_globalImports.IsDefault) + { + ImmutableInterlocked.InterlockedInitialize(ref _globalImports, BindGlobalImports()); + } + + return _globalImports; + } + } private ImmutableArray BindGlobalImports() { @@ -1565,7 +1573,8 @@ private ImmutableArray BindGlobalImports() /// /// Global imports not including those from previous submissions. /// - private UsingsFromOptionsAndDiagnostics UsingsFromOptions => _usingsFromOptions.Value; + private UsingsFromOptionsAndDiagnostics UsingsFromOptions + => InterlockedOperations.Initialize(ref _usingsFromOptions, static self => self.BindUsingsFromOptions(), this); private UsingsFromOptionsAndDiagnostics BindUsingsFromOptions() => UsingsFromOptionsAndDiagnostics.FromOptions(this); @@ -1590,7 +1599,8 @@ internal Imports GetSubmissionImports() /// /// Imports from all previous submissions. /// - internal Imports GetPreviousSubmissionImports() => _previousSubmissionImports.Value; + internal Imports GetPreviousSubmissionImports() + => InterlockedOperations.Initialize(ref _previousSubmissionImports, static self => self.ExpandPreviousSubmissionImports(), this); private Imports ExpandPreviousSubmissionImports() { @@ -1607,12 +1617,7 @@ private Imports ExpandPreviousSubmissionImports() } internal AliasSymbol GlobalNamespaceAlias - { - get - { - return _globalNamespaceAlias.Value; - } - } + => InterlockedOperations.Initialize(ref _globalNamespaceAlias, static self => self.CreateGlobalNamespaceAlias(), this); /// /// Get the symbol for the predefined type from the COR Library referenced by this compilation. From af60985c8c4ab1654a1a86e53c18561526cdde31 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Sun, 3 Mar 2024 06:56:37 -0800 Subject: [PATCH 2/7] Move a couple other fields initialization out of the ctor --- .../Operators/BinaryOperatorEasyOut.cs | 2 +- .../BinaryOperatorOverloadResolution.cs | 4 +-- .../Operators/UnaryOperatorEasyOut.cs | 2 +- .../UnaryOperatorOverloadResolution.cs | 2 +- .../Portable/Compilation/CSharpCompilation.cs | 31 +++++++++---------- .../Symbols/Compilation_WellKnownMembers.cs | 5 ++- .../Test/Emit2/Emit/NumericIntPtrTests.cs | 4 +-- .../Semantic/Semantics/NativeIntegerTests.cs | 4 +-- .../Test/Semantic/Semantics/OperatorTests.cs | 8 ++--- 9 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorEasyOut.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorEasyOut.cs index 0ec693b8156ff..a54c72b957ac3 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorEasyOut.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorEasyOut.cs @@ -309,7 +309,7 @@ private void BinaryOperatorEasyOut(BinaryOperatorKind kind, BoundExpression left return; } - BinaryOperatorSignature signature = this.Compilation.builtInOperators.GetSignature(easyOut); + BinaryOperatorSignature signature = this.Compilation.BuiltInOperators.GetSignature(easyOut); Conversion leftConversion = Conversions.FastClassifyConversion(leftType, signature.LeftType); Conversion rightConversion = Conversions.FastClassifyConversion(rightType, signature.RightType); diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs index 8895af98195d4..4f85925b77db2 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs @@ -719,7 +719,7 @@ private void GetAllBuiltInOperators(BinaryOperatorKind kind, bool isChecked, Bou } else { - this.Compilation.builtInOperators.GetSimpleBuiltInOperators(kind, operators, skipNativeIntegerOperators: !left.Type.IsNativeIntegerOrNullableThereof() && !right.Type.IsNativeIntegerOrNullableThereof()); + this.Compilation.BuiltInOperators.GetSimpleBuiltInOperators(kind, operators, skipNativeIntegerOperators: !left.Type.IsNativeIntegerOrNullableThereof() && !right.Type.IsNativeIntegerOrNullableThereof()); // SPEC 7.3.4: For predefined enum and delegate operators, the only operators // considered are those defined by an enum or delegate type that is the binding @@ -734,7 +734,7 @@ private void GetAllBuiltInOperators(BinaryOperatorKind kind, bool isChecked, Bou isUtf8ByteRepresentation(left) && isUtf8ByteRepresentation(right)) { - this.Compilation.builtInOperators.GetUtf8ConcatenationBuiltInOperator(left.Type, operators); + this.Compilation.BuiltInOperators.GetUtf8ConcatenationBuiltInOperator(left.Type, operators); } } diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorEasyOut.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorEasyOut.cs index 3fb09576b41a7..60dcb37adad9a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorEasyOut.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorEasyOut.cs @@ -109,7 +109,7 @@ private void UnaryOperatorEasyOut(UnaryOperatorKind kind, BoundExpression operan return; } - UnaryOperatorSignature signature = this.Compilation.builtInOperators.GetSignature(easyOut); + UnaryOperatorSignature signature = this.Compilation.BuiltInOperators.GetSignature(easyOut); Conversion? conversion = Conversions.FastClassifyConversion(operandType, signature.OperandType); diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs index a45f5438b9a81..1d7e91ec1f7e0 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs @@ -252,7 +252,7 @@ private void GetAllBuiltInOperators(UnaryOperatorKind kind, bool isChecked, Boun // specification to match the previous implementation. var operators = ArrayBuilder.GetInstance(); - this.Compilation.builtInOperators.GetSimpleBuiltInOperators(kind, operators, skipNativeIntegerOperators: !operand.Type.IsNativeIntegerOrNullableThereof()); + this.Compilation.BuiltInOperators.GetSimpleBuiltInOperators(kind, operators, skipNativeIntegerOperators: !operand.Type.IsNativeIntegerOrNullableThereof()); GetEnumOperations(kind, operand, operators); diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 0cf6d3743e772..21ae53ed073d1 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -84,11 +84,11 @@ internal Conversions Conversions /// /// Manages anonymous types declared in this compilation. Unifies types that are structurally equivalent. /// - private readonly AnonymousTypeManager _anonymousTypeManager; + private AnonymousTypeManager? _anonymousTypeManager; private NamespaceSymbol? _lazyGlobalNamespace; - internal readonly BuiltInOperators builtInOperators; + private BuiltInOperators? _builtInOperators; /// /// The for this compilation. Do not access directly, use Assembly property @@ -144,7 +144,7 @@ internal Conversions Conversions /// /// Cache of T to Nullable<T>. /// - private readonly ConcurrentCache _typeToNullableVersion = new ConcurrentCache(size: 100); + private ConcurrentCache? _typeToNullableVersion; /// Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor (temporary compat behavior). /// Must be removed prior to interceptors stable release. @@ -180,13 +180,11 @@ public override bool IsCaseSensitive } } + internal BuiltInOperators BuiltInOperators + => InterlockedOperations.Initialize(ref _builtInOperators, static self => new BuiltInOperators(self), this); + internal AnonymousTypeManager AnonymousTypeManager - { - get - { - return _anonymousTypeManager; - } - } + => InterlockedOperations.Initialize(ref _anonymousTypeManager, self => new AnonymousTypeManager(self), this); internal override CommonAnonymousTypeManager CommonAnonymousTypeManager { @@ -461,11 +459,8 @@ private CSharpCompilation( AsyncQueue? eventQueue = null) : base(assemblyName, references, features, isSubmission, semanticModelProvider, eventQueue) { - WellKnownMemberSignatureComparer = new WellKnownMembersSignatureComparer(this); _options = options; - this.builtInOperators = new BuiltInOperators(this); - _anonymousTypeManager = new AnonymousTypeManager(this); this.LanguageVersion = CommonLanguageVersion(syntaxAndDeclarations.ExternalSyntaxTrees); if (isSubmission) @@ -1644,6 +1639,9 @@ internal AliasSymbol GlobalNamespaceAlias return result; } + private ConcurrentCache TypeToNullableVersion + => InterlockedOperations.Initialize(ref _typeToNullableVersion, static () => new ConcurrentCache(size: 100)); + /// /// Given a provided , gives back constructed with that /// argument. This function is only intended to be used for very common instantiations produced heavily during @@ -1658,10 +1656,11 @@ internal NamedTypeSymbol GetOrCreateNullableType(TypeSymbol typeArgument) Debug.Fail($"Unsupported type argument: {typeArgument.ToDisplayString()}"); #endif - if (!_typeToNullableVersion.TryGetValue(typeArgument, out var constructedNullableInstance)) + var typeToNullableVersion = TypeToNullableVersion; + if (!typeToNullableVersion.TryGetValue(typeArgument, out var constructedNullableInstance)) { constructedNullableInstance = this.GetSpecialType(SpecialType.System_Nullable_T).Construct(typeArgument); - _typeToNullableVersion.TryAdd(typeArgument, constructedNullableInstance); + typeToNullableVersion.TryAdd(typeArgument, constructedNullableInstance); } return constructedNullableInstance; @@ -4203,7 +4202,7 @@ csharpLeftType.TypeKind is TypeKind.Dynamic || if (easyOutBinaryKind != BinaryOperatorKind.Error) { - var signature = this.builtInOperators.GetSignature(easyOutBinaryKind); + var signature = this.BuiltInOperators.GetSignature(easyOutBinaryKind); if (csharpReturnType.SpecialType == signature.ReturnType.SpecialType && csharpLeftType.SpecialType == signature.LeftType.SpecialType && csharpRightType.SpecialType == signature.RightType.SpecialType) @@ -4426,7 +4425,7 @@ void validateSignature() if (easyOutUnaryKind != UnaryOperatorKind.Error) { - var signature = this.builtInOperators.GetSignature(easyOutUnaryKind); + var signature = this.BuiltInOperators.GetSignature(easyOutUnaryKind); if (csharpReturnType.SpecialType == signature.ReturnType.SpecialType && csharpOperandType.SpecialType == signature.OperandType.SpecialType) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs index 8689b03637478..91eaa46cd5023 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp { public partial class CSharpCompilation { - internal readonly WellKnownMembersSignatureComparer WellKnownMemberSignatureComparer; + private WellKnownMembersSignatureComparer? _wellKnownMemberSignatureComparer; /// /// An array of cached well known types available for use in this Compilation. @@ -35,6 +35,9 @@ public partial class CSharpCompilation private int _needsGeneratedAttributes; private bool _needsGeneratedAttributes_IsFrozen; + internal WellKnownMembersSignatureComparer WellKnownMemberSignatureComparer + => InterlockedOperations.Initialize(ref _wellKnownMemberSignatureComparer, static self => new WellKnownMembersSignatureComparer(self), this); + /// /// Returns a value indicating which embedded attributes should be generated during emit phase. /// The value is set during binding the symbols that need those attributes, and is frozen on first trial to get it. diff --git a/src/Compilers/CSharp/Test/Emit2/Emit/NumericIntPtrTests.cs b/src/Compilers/CSharp/Test/Emit2/Emit/NumericIntPtrTests.cs index 8e1c7fe46abf2..4760ca1c2c254 100644 --- a/src/Compilers/CSharp/Test/Emit2/Emit/NumericIntPtrTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Emit/NumericIntPtrTests.cs @@ -1537,7 +1537,7 @@ static void verifyOperators(CSharpCompilation comp) static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind operatorKind, bool skipNativeIntegerOperators) { var builder = ArrayBuilder.GetInstance(); - comp.builtInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); + comp.BuiltInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); var operators = builder.ToImmutableAndFree(); int expectedSigned = skipNativeIntegerOperators ? 0 : 1; int expectedUnsigned = skipNativeIntegerOperators ? 0 : (operatorKind == UnaryOperatorKind.UnaryMinus) ? 0 : 1; @@ -1548,7 +1548,7 @@ static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind opera static void verifyBinaryOperators(CSharpCompilation comp, BinaryOperatorKind operatorKind, bool skipNativeIntegerOperators) { var builder = ArrayBuilder.GetInstance(); - comp.builtInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); + comp.BuiltInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); var operators = builder.ToImmutableAndFree(); int expected = skipNativeIntegerOperators ? 0 : 1; verifyOperators(operators, (op, signed) => isNativeInt(op.LeftType, signed), expected, expected); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs index 4c74fefb3c406..320f13951a7f9 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs @@ -4789,7 +4789,7 @@ static void verifyOperators(CSharpCompilation comp) static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind operatorKind, bool skipNativeIntegerOperators) { var builder = ArrayBuilder.GetInstance(); - comp.builtInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); + comp.BuiltInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); var operators = builder.ToImmutableAndFree(); int expectedSigned = skipNativeIntegerOperators ? 0 : 1; int expectedUnsigned = skipNativeIntegerOperators ? 0 : (operatorKind == UnaryOperatorKind.UnaryMinus) ? 0 : 1; @@ -4800,7 +4800,7 @@ static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind opera static void verifyBinaryOperators(CSharpCompilation comp, BinaryOperatorKind operatorKind, bool skipNativeIntegerOperators) { var builder = ArrayBuilder.GetInstance(); - comp.builtInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); + comp.BuiltInOperators.GetSimpleBuiltInOperators(operatorKind, builder, skipNativeIntegerOperators); var operators = builder.ToImmutableAndFree(); int expected = skipNativeIntegerOperators ? 0 : 1; verifyOperators(operators, (op, signed) => isNativeInt(op.LeftType, signed), expected, expected); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OperatorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OperatorTests.cs index f05146a2ca750..b31d4f8291d41 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OperatorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OperatorTests.cs @@ -7593,7 +7593,7 @@ ExpressionSyntax node4 } else { - signature = compilation.builtInOperators.GetSignature(result); + signature = compilation.BuiltInOperators.GetSignature(result); if ((object)underlying != (object)type) { @@ -8232,14 +8232,14 @@ ExpressionSyntax node8 else if ((op == BinaryOperatorKind.Addition || op == BinaryOperatorKind.Subtraction) && leftType.IsEnumType() && (rightType.IsIntegralType() || rightType.IsCharType()) && (result = OverloadResolution.BinopEasyOut.OpKind(op, leftType.EnumUnderlyingTypeOrSelf(), rightType)) != BinaryOperatorKind.Error && - TypeSymbol.Equals((signature = compilation.builtInOperators.GetSignature(result)).RightType, leftType.EnumUnderlyingTypeOrSelf(), TypeCompareKind.ConsiderEverything2)) + TypeSymbol.Equals((signature = compilation.BuiltInOperators.GetSignature(result)).RightType, leftType.EnumUnderlyingTypeOrSelf(), TypeCompareKind.ConsiderEverything2)) { signature = new BinaryOperatorSignature(signature.Kind | BinaryOperatorKind.EnumAndUnderlying, leftType, signature.RightType, leftType); } else if ((op == BinaryOperatorKind.Addition || op == BinaryOperatorKind.Subtraction) && rightType.IsEnumType() && (leftType.IsIntegralType() || leftType.IsCharType()) && (result = OverloadResolution.BinopEasyOut.OpKind(op, leftType, rightType.EnumUnderlyingTypeOrSelf())) != BinaryOperatorKind.Error && - TypeSymbol.Equals((signature = compilation.builtInOperators.GetSignature(result)).LeftType, rightType.EnumUnderlyingTypeOrSelf(), TypeCompareKind.ConsiderEverything2)) + TypeSymbol.Equals((signature = compilation.BuiltInOperators.GetSignature(result)).LeftType, rightType.EnumUnderlyingTypeOrSelf(), TypeCompareKind.ConsiderEverything2)) { signature = new BinaryOperatorSignature(signature.Kind | BinaryOperatorKind.EnumAndUnderlying, signature.LeftType, rightType, rightType); } @@ -8355,7 +8355,7 @@ ExpressionSyntax node8 } else { - signature = compilation.builtInOperators.GetSignature(result); + signature = compilation.BuiltInOperators.GetSignature(result); } Assert.NotNull(symbol1); From 8da54e8fc7f32dfd2e840b5e2d026184ec0d7087 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 4 Mar 2024 11:48:20 -0800 Subject: [PATCH 3/7] add static keyword for clarity Reorder bool assignment and CompareExchange to avoid potential usage of member when not initialized --- .../Portable/Compilation/CSharpCompilation.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 21ae53ed073d1..58f65f1f8b4b5 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -184,7 +184,7 @@ internal BuiltInOperators BuiltInOperators => InterlockedOperations.Initialize(ref _builtInOperators, static self => new BuiltInOperators(self), this); internal AnonymousTypeManager AnonymousTypeManager - => InterlockedOperations.Initialize(ref _anonymousTypeManager, self => new AnonymousTypeManager(self), this); + => InterlockedOperations.Initialize(ref _anonymousTypeManager, static self => new AnonymousTypeManager(self), this); internal override CommonAnonymousTypeManager CommonAnonymousTypeManager { @@ -1490,8 +1490,8 @@ internal bool GetExternAliasTarget(string aliasName, out NamespaceSymbol @namesp // _scriptClass is allowed to be null, thus why a bool is used to track initialization if (!_scriptClassInitialized) { - _scriptClassInitialized = true; Interlocked.CompareExchange(ref _scriptClass, BindScriptClass(), null); + _scriptClassInitialized = true; } return _scriptClass; @@ -1519,17 +1519,7 @@ internal bool IsSubmissionSyntaxTree(SyntaxTree tree) /// Global imports (including those from previous submissions, if there are any). /// internal ImmutableArray GlobalImports - { - get - { - if (_globalImports.IsDefault) - { - ImmutableInterlocked.InterlockedInitialize(ref _globalImports, BindGlobalImports()); - } - - return _globalImports; - } - } + => InterlockedOperations.Initialize(ref _globalImports, () => BindGlobalImports()); private ImmutableArray BindGlobalImports() { From 5d94e883011604a06c5eecd698cc2a8b14f9613d Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 4 Mar 2024 12:34:45 -0800 Subject: [PATCH 4/7] use StrongBox instead of adding a bool --- .../Portable/Compilation/CSharpCompilation.cs | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 58f65f1f8b4b5..0ad1046e749d0 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -12,6 +12,7 @@ using System.Linq; using System.Reflection; using System.Reflection.Metadata; +using System.Runtime.CompilerServices; using System.Threading; using Microsoft.Cci; using Microsoft.CodeAnalysis; @@ -55,8 +56,7 @@ public sealed partial class CSharpCompilation : Compilation private Imports? _previousSubmissionImports; private AliasSymbol? _globalNamespaceAlias; // alias symbol used to resolve "global::". - private bool _scriptClassInitialized; - private ImplicitNamedTypeSymbol? _scriptClass; + private StrongBox? _scriptClass; // The type of host object model if available. private TypeSymbol? _lazyHostObjectTypeSymbol; @@ -1484,19 +1484,7 @@ internal bool GetExternAliasTarget(string aliasName, out NamespaceSymbol @namesp /// defined in the compilation. /// internal new NamedTypeSymbol? ScriptClass - { - get - { - // _scriptClass is allowed to be null, thus why a bool is used to track initialization - if (!_scriptClassInitialized) - { - Interlocked.CompareExchange(ref _scriptClass, BindScriptClass(), null); - _scriptClassInitialized = true; - } - - return _scriptClass; - } - } + => InterlockedOperations.Initialize(ref _scriptClass, static self => self.BindScriptClass(), this); /// /// Resolves a symbol that represents script container (Script class). Uses the @@ -1519,7 +1507,7 @@ internal bool IsSubmissionSyntaxTree(SyntaxTree tree) /// Global imports (including those from previous submissions, if there are any). /// internal ImmutableArray GlobalImports - => InterlockedOperations.Initialize(ref _globalImports, () => BindGlobalImports()); + => InterlockedOperations.Initialize(ref _globalImports, static self => self.BindGlobalImports(), arg: this); private ImmutableArray BindGlobalImports() { From 0836e9bf415c0646faf93f171808b2abbdae1ced Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 5 Mar 2024 09:09:37 -0800 Subject: [PATCH 5/7] Maintain original formatting Use lazy naming convention Move away from StrongBox usage --- .../Portable/Compilation/CSharpCompilation.cs | 70 +++++++++++++++---- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 0ad1046e749d0..ca36678c9c8aa 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -12,7 +12,6 @@ using System.Linq; using System.Reflection; using System.Reflection.Metadata; -using System.Runtime.CompilerServices; using System.Threading; using Microsoft.Cci; using Microsoft.CodeAnalysis; @@ -51,12 +50,12 @@ public sealed partial class CSharpCompilation : Compilation // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! private readonly CSharpCompilationOptions _options; - private UsingsFromOptionsAndDiagnostics? _usingsFromOptions; - private ImmutableArray _globalImports; - private Imports? _previousSubmissionImports; - private AliasSymbol? _globalNamespaceAlias; // alias symbol used to resolve "global::". + private UsingsFromOptionsAndDiagnostics? _lazyUsingsFromOptions; + private ImmutableArray _lazyGlobalImports; + private Imports? _lazyPreviousSubmissionImports; + private AliasSymbol? _lazyGlobalNamespaceAlias; // alias symbol used to resolve "global::". - private StrongBox? _scriptClass; + private NamedTypeSymbol? _lazyScriptClass; // The type of host object model if available. private TypeSymbol? _lazyHostObjectTypeSymbol; @@ -79,7 +78,17 @@ public sealed partial class CSharpCompilation : Compilation /// A conversions object that ignores nullability. /// internal Conversions Conversions - => InterlockedOperations.Initialize(ref _conversions, static self => new BuckStopsHereBinder(self, associatedFileIdentifier: null).Conversions, this); + { + get + { + if (_conversions == null) + { + Interlocked.CompareExchange(ref _conversions, new BuckStopsHereBinder(this, associatedFileIdentifier: null).Conversions, null); + } + + return _conversions; + } + } /// /// Manages anonymous types declared in this compilation. Unifies types that are structurally equivalent. @@ -181,10 +190,20 @@ public override bool IsCaseSensitive } internal BuiltInOperators BuiltInOperators - => InterlockedOperations.Initialize(ref _builtInOperators, static self => new BuiltInOperators(self), this); + { + get + { + return InterlockedOperations.Initialize(ref _builtInOperators, static self => new BuiltInOperators(self), this); + } + } internal AnonymousTypeManager AnonymousTypeManager - => InterlockedOperations.Initialize(ref _anonymousTypeManager, static self => new AnonymousTypeManager(self), this); + { + get + { + return InterlockedOperations.Initialize(ref _anonymousTypeManager, static self => new AnonymousTypeManager(self), this); + } + } internal override CommonAnonymousTypeManager CommonAnonymousTypeManager { @@ -460,6 +479,7 @@ private CSharpCompilation( : base(assemblyName, references, features, isSubmission, semanticModelProvider, eventQueue) { _options = options; + _lazyScriptClass = ErrorTypeSymbol.UnknownResultType; this.LanguageVersion = CommonLanguageVersion(syntaxAndDeclarations.ExternalSyntaxTrees); @@ -1484,7 +1504,17 @@ internal bool GetExternAliasTarget(string aliasName, out NamespaceSymbol @namesp /// defined in the compilation. /// internal new NamedTypeSymbol? ScriptClass - => InterlockedOperations.Initialize(ref _scriptClass, static self => self.BindScriptClass(), this); + { + get + { + if (ReferenceEquals(_lazyScriptClass, ErrorTypeSymbol.UnknownResultType)) + { + Interlocked.CompareExchange(ref _lazyScriptClass, BindScriptClass()!, ErrorTypeSymbol.UnknownResultType); + } + + return _lazyScriptClass; + } + } /// /// Resolves a symbol that represents script container (Script class). Uses the @@ -1507,7 +1537,7 @@ internal bool IsSubmissionSyntaxTree(SyntaxTree tree) /// Global imports (including those from previous submissions, if there are any). /// internal ImmutableArray GlobalImports - => InterlockedOperations.Initialize(ref _globalImports, static self => self.BindGlobalImports(), arg: this); + => InterlockedOperations.Initialize(ref _lazyGlobalImports, static self => self.BindGlobalImports(), arg: this); private ImmutableArray BindGlobalImports() { @@ -1547,7 +1577,7 @@ private ImmutableArray BindGlobalImports() /// Global imports not including those from previous submissions. /// private UsingsFromOptionsAndDiagnostics UsingsFromOptions - => InterlockedOperations.Initialize(ref _usingsFromOptions, static self => self.BindUsingsFromOptions(), this); + => InterlockedOperations.Initialize(ref _lazyUsingsFromOptions, static self => self.BindUsingsFromOptions(), this); private UsingsFromOptionsAndDiagnostics BindUsingsFromOptions() => UsingsFromOptionsAndDiagnostics.FromOptions(this); @@ -1573,7 +1603,7 @@ internal Imports GetSubmissionImports() /// Imports from all previous submissions. /// internal Imports GetPreviousSubmissionImports() - => InterlockedOperations.Initialize(ref _previousSubmissionImports, static self => self.ExpandPreviousSubmissionImports(), this); + => InterlockedOperations.Initialize(ref _lazyPreviousSubmissionImports, static self => self.ExpandPreviousSubmissionImports(), this); private Imports ExpandPreviousSubmissionImports() { @@ -1590,7 +1620,12 @@ private Imports ExpandPreviousSubmissionImports() } internal AliasSymbol GlobalNamespaceAlias - => InterlockedOperations.Initialize(ref _globalNamespaceAlias, static self => self.CreateGlobalNamespaceAlias(), this); + { + get + { + return InterlockedOperations.Initialize(ref _lazyGlobalNamespaceAlias, static self => self.CreateGlobalNamespaceAlias(), this); + } + } /// /// Get the symbol for the predefined type from the COR Library referenced by this compilation. @@ -1618,7 +1653,12 @@ internal AliasSymbol GlobalNamespaceAlias } private ConcurrentCache TypeToNullableVersion - => InterlockedOperations.Initialize(ref _typeToNullableVersion, static () => new ConcurrentCache(size: 100)); + { + get + { + return InterlockedOperations.Initialize(ref _typeToNullableVersion, static () => new ConcurrentCache(size: 100)); + } + } /// /// Given a provided , gives back constructed with that From 3f75750a941f2078d851cfc1d32d869732d3ccfc Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 6 Mar 2024 11:44:04 -0800 Subject: [PATCH 6/7] Rename a couple fields to indicate lazy --- .../Portable/Compilation/CSharpCompilation.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index ca36678c9c8aa..18c0cf52a71d2 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -55,7 +55,7 @@ public sealed partial class CSharpCompilation : Compilation private Imports? _lazyPreviousSubmissionImports; private AliasSymbol? _lazyGlobalNamespaceAlias; // alias symbol used to resolve "global::". - private NamedTypeSymbol? _lazyScriptClass; + private NamedTypeSymbol? _lazyScriptClass = ErrorTypeSymbol.UnknownResultType; // The type of host object model if available. private TypeSymbol? _lazyHostObjectTypeSymbol; @@ -93,11 +93,11 @@ internal Conversions Conversions /// /// Manages anonymous types declared in this compilation. Unifies types that are structurally equivalent. /// - private AnonymousTypeManager? _anonymousTypeManager; + private AnonymousTypeManager? _lazyAnonymousTypeManager; private NamespaceSymbol? _lazyGlobalNamespace; - private BuiltInOperators? _builtInOperators; + private BuiltInOperators? _lazyBuiltInOperators; /// /// The for this compilation. Do not access directly, use Assembly property @@ -153,7 +153,7 @@ internal Conversions Conversions /// /// Cache of T to Nullable<T>. /// - private ConcurrentCache? _typeToNullableVersion; + private ConcurrentCache? _lazyTypeToNullableVersion; /// Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor (temporary compat behavior). /// Must be removed prior to interceptors stable release. @@ -193,7 +193,7 @@ internal BuiltInOperators BuiltInOperators { get { - return InterlockedOperations.Initialize(ref _builtInOperators, static self => new BuiltInOperators(self), this); + return InterlockedOperations.Initialize(ref _lazyBuiltInOperators, static self => new BuiltInOperators(self), this); } } @@ -201,7 +201,7 @@ internal AnonymousTypeManager AnonymousTypeManager { get { - return InterlockedOperations.Initialize(ref _anonymousTypeManager, static self => new AnonymousTypeManager(self), this); + return InterlockedOperations.Initialize(ref _lazyAnonymousTypeManager, static self => new AnonymousTypeManager(self), this); } } @@ -479,7 +479,6 @@ private CSharpCompilation( : base(assemblyName, references, features, isSubmission, semanticModelProvider, eventQueue) { _options = options; - _lazyScriptClass = ErrorTypeSymbol.UnknownResultType; this.LanguageVersion = CommonLanguageVersion(syntaxAndDeclarations.ExternalSyntaxTrees); @@ -1656,7 +1655,7 @@ private ConcurrentCache TypeToNullableVersion { get { - return InterlockedOperations.Initialize(ref _typeToNullableVersion, static () => new ConcurrentCache(size: 100)); + return InterlockedOperations.Initialize(ref _lazyTypeToNullableVersion, static () => new ConcurrentCache(size: 100)); } } From 492465c232f709f8f73d3b2c60e7e7011f86bde2 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 6 Mar 2024 15:12:13 -0800 Subject: [PATCH 7/7] Another switch to include lazy in the field name --- .../CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs index 91eaa46cd5023..a2f9383db8641 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp { public partial class CSharpCompilation { - private WellKnownMembersSignatureComparer? _wellKnownMemberSignatureComparer; + private WellKnownMembersSignatureComparer? _lazyWellKnownMemberSignatureComparer; /// /// An array of cached well known types available for use in this Compilation. @@ -36,7 +36,7 @@ public partial class CSharpCompilation private bool _needsGeneratedAttributes_IsFrozen; internal WellKnownMembersSignatureComparer WellKnownMemberSignatureComparer - => InterlockedOperations.Initialize(ref _wellKnownMemberSignatureComparer, static self => new WellKnownMembersSignatureComparer(self), this); + => InterlockedOperations.Initialize(ref _lazyWellKnownMemberSignatureComparer, static self => new WellKnownMembersSignatureComparer(self), this); /// /// Returns a value indicating which embedded attributes should be generated during emit phase.