Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce closure and Lazy allocations inside CSharpCompilation.ctor #72371

Merged
merged 7 commits into from
Mar 7, 2024
Merged

Reduce closure and Lazy allocations inside CSharpCompilation.ctor #72371

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
89ac45d
Reduce closure and Lazy allocations inside CSharpCompilation.ctor
ToddGrun Mar 2, 2024
af60985
Move a couple other fields initialization out of the ctor
ToddGrun Mar 3, 2024
8da54e8
add static keyword for clarity
ToddGrun Mar 4, 2024
5d94e88
use StrongBox instead of adding a bool
ToddGrun Mar 4, 2024
0836e9b
Maintain original formatting
ToddGrun Mar 5, 2024
3f75750
Rename a couple fields to indicate lazy
ToddGrun Mar 6, 2024
492465c
Another switch to include lazy in the field name
ToddGrun Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorEasyOut.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorEasyOut.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ private void GetAllBuiltInOperators(UnaryOperatorKind kind, bool isChecked, Boun
// specification to match the previous implementation.

var operators = ArrayBuilder<UnaryOperatorSignature>.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);

Expand Down
88 changes: 41 additions & 47 deletions src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ public sealed partial class CSharpCompilation : Compilation
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

private readonly CSharpCompilationOptions _options;
private readonly Lazy<UsingsFromOptionsAndDiagnostics> _usingsFromOptions;
private readonly Lazy<ImmutableArray<NamespaceOrTypeAndUsingDirective>> _globalImports;
private readonly Lazy<Imports> _previousSubmissionImports;
private readonly Lazy<AliasSymbol> _globalNamespaceAlias; // alias symbol used to resolve "global::".
private readonly Lazy<ImplicitNamedTypeSymbol?> _scriptClass;
private UsingsFromOptionsAndDiagnostics? _usingsFromOptions;
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved
private ImmutableArray<NamespaceOrTypeAndUsingDirective> _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;
Expand All @@ -77,26 +79,16 @@ public sealed partial class CSharpCompilation : Compilation
/// A conversions object that ignores nullability.
/// </summary>
internal Conversions Conversions
{
get
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved
{
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);
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Manages anonymous types declared in this compilation. Unifies types that are structurally equivalent.
/// </summary>
private readonly AnonymousTypeManager _anonymousTypeManager;
private AnonymousTypeManager? _anonymousTypeManager;
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_anonymousTypeManager

lazy? #Closed


private NamespaceSymbol? _lazyGlobalNamespace;

internal readonly BuiltInOperators builtInOperators;
private BuiltInOperators? _builtInOperators;
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_builtInOperators

lazy? #Closed


/// <summary>
/// The <see cref="SourceAssemblySymbol"/> for this compilation. Do not access directly, use Assembly property
Expand Down Expand Up @@ -152,7 +144,7 @@ internal Conversions Conversions
/// <summary>
/// Cache of T to Nullable&lt;T&gt;.
/// </summary>
private readonly ConcurrentCache<TypeSymbol, NamedTypeSymbol> _typeToNullableVersion = new ConcurrentCache<TypeSymbol, NamedTypeSymbol>(size: 100);
private ConcurrentCache<TypeSymbol, NamedTypeSymbol>? _typeToNullableVersion;
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_typeToNullableVersion

lazy? #Closed


/// <summary>Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor (temporary compat behavior).</summary>
/// <remarks>Must be removed prior to interceptors stable release.</remarks>
Expand Down Expand Up @@ -188,13 +180,11 @@ public override bool IsCaseSensitive
}
}

internal BuiltInOperators BuiltInOperators
=> InterlockedOperations.Initialize(ref _builtInOperators, static self => new BuiltInOperators(self), this);

internal AnonymousTypeManager AnonymousTypeManager
{
get
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved
{
return _anonymousTypeManager;
}
}
=> InterlockedOperations.Initialize(ref _anonymousTypeManager, static self => new AnonymousTypeManager(self), this);

internal override CommonAnonymousTypeManager CommonAnonymousTypeManager
{
Expand Down Expand Up @@ -469,16 +459,8 @@ private CSharpCompilation(
AsyncQueue<CompilationEvent>? eventQueue = null)
: base(assemblyName, references, features, isSubmission, semanticModelProvider, eventQueue)
{
WellKnownMemberSignatureComparer = new WellKnownMembersSignatureComparer(this);
_options = options;

this.builtInOperators = new BuiltInOperators(this);
_scriptClass = new Lazy<ImplicitNamedTypeSymbol?>(BindScriptClass);
_globalImports = new Lazy<ImmutableArray<NamespaceOrTypeAndUsingDirective>>(BindGlobalImports);
_usingsFromOptions = new Lazy<UsingsFromOptionsAndDiagnostics>(BindUsingsFromOptions);
_previousSubmissionImports = new Lazy<Imports>(ExpandPreviousSubmissionImports);
_globalNamespaceAlias = new Lazy<AliasSymbol>(CreateGlobalNamespaceAlias);
_anonymousTypeManager = new AnonymousTypeManager(this);
this.LanguageVersion = CommonLanguageVersion(syntaxAndDeclarations.ExternalSyntaxTrees);

if (isSubmission)
Expand Down Expand Up @@ -1503,7 +1485,17 @@ internal bool GetExternAliasTarget(string aliasName, out NamespaceSymbol @namesp
/// </summary>
internal new NamedTypeSymbol? ScriptClass
{
get { return _scriptClass.Value; }
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved
get
{
// _scriptClass is allowed to be null, thus why a bool is used to track initialization
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved
if (!_scriptClassInitialized)
{
Interlocked.CompareExchange(ref _scriptClass, BindScriptClass(), null);
_scriptClassInitialized = true;
}

return _scriptClass;
}
}

/// <summary>
Expand All @@ -1526,7 +1518,8 @@ internal bool IsSubmissionSyntaxTree(SyntaxTree tree)
/// <summary>
/// Global imports (including those from previous submissions, if there are any).
/// </summary>
internal ImmutableArray<NamespaceOrTypeAndUsingDirective> GlobalImports => _globalImports.Value;
internal ImmutableArray<NamespaceOrTypeAndUsingDirective> GlobalImports
=> InterlockedOperations.Initialize(ref _globalImports, () => BindGlobalImports());
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved

private ImmutableArray<NamespaceOrTypeAndUsingDirective> BindGlobalImports()
{
Expand Down Expand Up @@ -1565,7 +1558,8 @@ private ImmutableArray<NamespaceOrTypeAndUsingDirective> BindGlobalImports()
/// <summary>
/// Global imports not including those from previous submissions.
/// </summary>
private UsingsFromOptionsAndDiagnostics UsingsFromOptions => _usingsFromOptions.Value;
private UsingsFromOptionsAndDiagnostics UsingsFromOptions
=> InterlockedOperations.Initialize(ref _usingsFromOptions, static self => self.BindUsingsFromOptions(), this);

private UsingsFromOptionsAndDiagnostics BindUsingsFromOptions() => UsingsFromOptionsAndDiagnostics.FromOptions(this);

Expand All @@ -1590,7 +1584,8 @@ internal Imports GetSubmissionImports()
/// <summary>
/// Imports from all previous submissions.
/// </summary>
internal Imports GetPreviousSubmissionImports() => _previousSubmissionImports.Value;
internal Imports GetPreviousSubmissionImports()
=> InterlockedOperations.Initialize(ref _previousSubmissionImports, static self => self.ExpandPreviousSubmissionImports(), this);

private Imports ExpandPreviousSubmissionImports()
{
Expand All @@ -1607,12 +1602,7 @@ private Imports ExpandPreviousSubmissionImports()
}

internal AliasSymbol GlobalNamespaceAlias
{
get
{
return _globalNamespaceAlias.Value;
}
}
=> InterlockedOperations.Initialize(ref _globalNamespaceAlias, static self => self.CreateGlobalNamespaceAlias(), this);

/// <summary>
/// Get the symbol for the predefined type from the COR Library referenced by this compilation.
Expand All @@ -1639,6 +1629,9 @@ internal AliasSymbol GlobalNamespaceAlias
return result;
}

private ConcurrentCache<TypeSymbol, NamedTypeSymbol> TypeToNullableVersion
=> InterlockedOperations.Initialize(ref _typeToNullableVersion, static () => new ConcurrentCache<TypeSymbol, NamedTypeSymbol>(size: 100));

/// <summary>
/// Given a provided <paramref name="typeArgument"/>, gives back <see cref="Nullable{T}"/> constructed with that
/// argument. This function is only intended to be used for very common instantiations produced heavily during
Expand All @@ -1653,10 +1646,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;
Expand Down Expand Up @@ -4198,7 +4192,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)
Expand Down Expand Up @@ -4421,7 +4415,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)
{
Expand Down
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp
{
public partial class CSharpCompilation
{
internal readonly WellKnownMembersSignatureComparer WellKnownMemberSignatureComparer;
private WellKnownMembersSignatureComparer? _wellKnownMemberSignatureComparer;
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// An array of cached well known types available for use in this Compilation.
Expand All @@ -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);

/// <summary>
/// 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.
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit2/Emit/NumericIntPtrTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1537,7 +1537,7 @@ static void verifyOperators(CSharpCompilation comp)
static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind operatorKind, bool skipNativeIntegerOperators)
{
var builder = ArrayBuilder<UnaryOperatorSignature>.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;
Expand All @@ -1548,7 +1548,7 @@ static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind opera
static void verifyBinaryOperators(CSharpCompilation comp, BinaryOperatorKind operatorKind, bool skipNativeIntegerOperators)
{
var builder = ArrayBuilder<BinaryOperatorSignature>.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);
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4789,7 +4789,7 @@ static void verifyOperators(CSharpCompilation comp)
static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind operatorKind, bool skipNativeIntegerOperators)
{
var builder = ArrayBuilder<UnaryOperatorSignature>.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;
Expand All @@ -4800,7 +4800,7 @@ static void verifyUnaryOperators(CSharpCompilation comp, UnaryOperatorKind opera
static void verifyBinaryOperators(CSharpCompilation comp, BinaryOperatorKind operatorKind, bool skipNativeIntegerOperators)
{
var builder = ArrayBuilder<BinaryOperatorSignature>.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);
Expand Down
Loading
Loading