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.

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

Use a singleton sentinel for empty type-parameters for non-generic named-types and methods. #68131

Merged
merged 13 commits into from
May 11, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
// That is, for a generic type C<T> this is the instance type C<T>.
internal sealed partial class SourceNamedTypeSymbol : SourceMemberContainerTypeSymbol, IAttributeTargetSymbol
{
private ImmutableArray<TypeParameterSymbol> _lazyTypeParameters;

/// <summary>
/// A collection of type parameter constraint types, populated when
/// constraint types for the first type parameter are requested.
/// </summary>
private ImmutableArray<ImmutableArray<TypeWithAnnotations>> _lazyTypeParameterConstraintTypes;

/// <summary>
/// A collection of type parameter constraint kinds, populated when
/// constraint kinds for the first type parameter are requested.
/// </summary>
private ImmutableArray<TypeParameterConstraintKind> _lazyTypeParameterConstraintKinds;
private readonly TypeParameterInfo _typeParameterInfo;

private CustomAttributesBag<CSharpAttributeData> _lazyCustomAttributesBag;

Expand Down Expand Up @@ -104,6 +92,10 @@ internal SourceNamedTypeSymbol(NamespaceOrTypeSymbol containingSymbol, MergedTyp
// Nested types are never unified.
_lazyIsExplicitDefinitionOfNoPiaLocalType = ThreeState.False;
}

_typeParameterInfo = declaration.Arity == 0
? TypeParameterInfo.Empty
: new TypeParameterInfo();
}

protected override NamedTypeSymbol WithTupleDataCore(TupleExtraData newData)
Expand Down Expand Up @@ -277,21 +269,22 @@ internal ImmutableArray<TypeWithAnnotations> GetTypeParameterConstraintTypes(int

private ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTypeParameterConstraintTypes()
{
var constraintTypes = _lazyTypeParameterConstraintTypes;
if (constraintTypes.IsDefault)
if (_typeParameterInfo.LazyTypeParameterConstraintTypes.IsDefault)
{
GetTypeParameterConstraintKinds();

var diagnostics = BindingDiagnosticBag.GetInstance();
if (ImmutableInterlocked.InterlockedInitialize(ref _lazyTypeParameterConstraintTypes, MakeTypeParameterConstraintTypes(diagnostics)))
if (ImmutableInterlocked.InterlockedInitialize(
ref _typeParameterInfo.LazyTypeParameterConstraintTypes,
MakeTypeParameterConstraintTypes(diagnostics)))
{
this.AddDeclarationDiagnostics(diagnostics);
}
diagnostics.Free();
constraintTypes = _lazyTypeParameterConstraintTypes;
}

return constraintTypes;
Debug.Assert(!_typeParameterInfo.LazyTypeParameterConstraintTypes.IsDefault);
return _typeParameterInfo.LazyTypeParameterConstraintTypes;
}

/// <summary>
Expand All @@ -305,14 +298,15 @@ internal TypeParameterConstraintKind GetTypeParameterConstraintKind(int ordinal)

private ImmutableArray<TypeParameterConstraintKind> GetTypeParameterConstraintKinds()
{
var constraintKinds = _lazyTypeParameterConstraintKinds;
if (constraintKinds.IsDefault)
if (_typeParameterInfo.LazyTypeParameterConstraintKinds.IsDefault)
{
ImmutableInterlocked.InterlockedInitialize(ref _lazyTypeParameterConstraintKinds, MakeTypeParameterConstraintKinds());
constraintKinds = _lazyTypeParameterConstraintKinds;
ImmutableInterlocked.InterlockedInitialize(
ref _typeParameterInfo.LazyTypeParameterConstraintKinds,
MakeTypeParameterConstraintKinds());
}

return constraintKinds;
Debug.Assert(!_typeParameterInfo.LazyTypeParameterConstraintKinds.IsDefault);
return _typeParameterInfo.LazyTypeParameterConstraintKinds;
}

private ImmutableArray<ImmutableArray<TypeWithAnnotations>> MakeTypeParameterConstraintTypes(BindingDiagnosticBag diagnostics)
Expand Down Expand Up @@ -749,18 +743,20 @@ public override ImmutableArray<TypeParameterSymbol> TypeParameters
{
get
{
if (_lazyTypeParameters.IsDefault)
if (_typeParameterInfo.LazyTypeParameters.IsDefault)
{
var diagnostics = BindingDiagnosticBag.GetInstance();
if (ImmutableInterlocked.InterlockedInitialize(ref _lazyTypeParameters, MakeTypeParameters(diagnostics)))
if (ImmutableInterlocked.InterlockedInitialize(
ref _typeParameterInfo.LazyTypeParameters,
MakeTypeParameters(diagnostics)))
{
AddDeclarationDiagnostics(diagnostics);
}

diagnostics.Free();
}

return _lazyTypeParameters;
return _typeParameterInfo.LazyTypeParameters;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Globalization;
Expand All @@ -25,19 +24,7 @@ internal sealed class SourceOrdinaryMethodSymbol : SourceOrdinaryMethodSymbolBas
private readonly RefKind _refKind;
private bool _lazyIsVararg;

/// <summary>
/// A collection of type parameter constraint types, populated when
/// constraint types for the first type parameter is requested.
/// Initialized in two steps. Hold a copy if accessing during initialization.
/// </summary>
private ImmutableArray<ImmutableArray<TypeWithAnnotations>> _lazyTypeParameterConstraintTypes;

/// <summary>
/// A collection of type parameter constraint kinds, populated when
/// constraint kinds for the first type parameter is requested.
/// Initialized in two steps. Hold a copy if accessing during initialization.
/// </summary>
private ImmutableArray<TypeParameterConstraintKind> _lazyTypeParameterConstraintKinds;
private readonly TypeParameterInfo _typeParameterInfo;

/// <summary>
/// If this symbol represents a partial method definition or implementation part, its other part (if any).
Expand Down Expand Up @@ -92,6 +79,8 @@ private SourceOrdinaryMethodSymbol(
{
Debug.Assert(diagnostics.DiagnosticBag is object);

_typeParameterInfo = TypeParameterInfo.Create(MakeTypeParameters(syntax, diagnostics));

_explicitInterfaceType = explicitInterfaceType;

bool hasBlockBody = syntax.Body != null;
Expand All @@ -105,17 +94,12 @@ private SourceOrdinaryMethodSymbol(
syntax.Body, syntax.ExpressionBody, syntax, diagnostics);
}

protected override ImmutableArray<TypeParameterSymbol> MakeTypeParameters(CSharpSyntaxNode node, BindingDiagnosticBag diagnostics)
public override ImmutableArray<TypeParameterSymbol> TypeParameters
{
var syntax = (MethodDeclarationSyntax)node;
if (syntax.Arity == 0)
{
ReportErrorIfHasConstraints(syntax.ConstraintClauses, diagnostics.DiagnosticBag);
return ImmutableArray<TypeParameterSymbol>.Empty;
}
else
get
{
return MakeTypeParameters(syntax, diagnostics);
Debug.Assert(!_typeParameterInfo.LazyTypeParameters.IsDefault);
return _typeParameterInfo.LazyTypeParameters;
}
}

Expand Down Expand Up @@ -308,7 +292,7 @@ protected override void CompleteAsyncMethodChecksBetweenStartAndFinish()

public override ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTypeParameterConstraintTypes()
{
if (_lazyTypeParameterConstraintTypes.IsDefault)
if (_typeParameterInfo.LazyTypeParameterConstraintTypes.IsDefault)
{
GetTypeParameterConstraintKinds();

Expand All @@ -324,19 +308,21 @@ public override ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTypeParam
syntax.TypeParameterList,
syntax.ConstraintClauses,
diagnostics);
if (ImmutableInterlocked.InterlockedInitialize(ref _lazyTypeParameterConstraintTypes, constraints))
if (ImmutableInterlocked.InterlockedInitialize(
ref _typeParameterInfo.LazyTypeParameterConstraintTypes,
constraints))
{
this.AddDeclarationDiagnostics(diagnostics);
}
diagnostics.Free();
}

return _lazyTypeParameterConstraintTypes;
return _typeParameterInfo.LazyTypeParameterConstraintTypes;
}

public override ImmutableArray<TypeParameterConstraintKind> GetTypeParameterConstraintKinds()
{
if (_lazyTypeParameterConstraintKinds.IsDefault)
if (_typeParameterInfo.LazyTypeParameterConstraintKinds.IsDefault)
{
var syntax = GetSyntax();
var withTypeParametersBinder =
Expand All @@ -349,10 +335,12 @@ public override ImmutableArray<TypeParameterConstraintKind> GetTypeParameterCons
syntax.TypeParameterList,
syntax.ConstraintClauses);

ImmutableInterlocked.InterlockedInitialize(ref _lazyTypeParameterConstraintKinds, constraints);
ImmutableInterlocked.InterlockedInitialize(
ref _typeParameterInfo.LazyTypeParameterConstraintKinds,
constraints);
}

return _lazyTypeParameterConstraintKinds;
return _typeParameterInfo.LazyTypeParameterConstraintKinds;
}

public override bool IsVararg
Expand Down Expand Up @@ -530,6 +518,12 @@ protected override DeclarationModifiers MakeDeclarationModifiers(DeclarationModi

private ImmutableArray<TypeParameterSymbol> MakeTypeParameters(MethodDeclarationSyntax syntax, BindingDiagnosticBag diagnostics)
{
if (syntax.Arity == 0)
{
ReportErrorIfHasConstraints(syntax.ConstraintClauses, diagnostics.DiagnosticBag);
return ImmutableArray<TypeParameterSymbol>.Empty;
}

Debug.Assert(syntax.TypeParameterList != null);

MessageID.IDS_FeatureGenerics.CheckFeatureAvailability(diagnostics, syntax.TypeParameterList.LessThanToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
/// </summary>
internal abstract class SourceOrdinaryMethodSymbolBase : SourceOrdinaryMethodOrUserDefinedOperatorSymbol
{
private readonly ImmutableArray<TypeParameterSymbol> _typeParameters;
Copy link
Member Author

Choose a reason for hiding this comment

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

methods were odd. _typeParameters was in SourceOrdinaryMethodSymbolBase (not SourceOrdinaryMethodSymbol), but was always empty for all other types. So this pushed this value down into SourceOrdinaryMethodSymbol, which could then use the new TypeParameterInfo object. All other methods just return 'Empty' from their TypeParameters helpers.

Note: this also ends up saving this extra pointer from all other source methods.

private readonly string _name;

protected SourceOrdinaryMethodSymbolBase(
Expand Down Expand Up @@ -62,8 +61,6 @@ protected SourceOrdinaryMethodSymbolBase(

this.MakeFlags(methodKind, declarationModifiers, returnsVoid, isExtensionMethod: isExtensionMethod, isNullableAnalysisEnabled: isNullableAnalysisEnabled, isMetadataVirtualIgnoringModifiers: isMetadataVirtualIgnoringModifiers);

_typeParameters = MakeTypeParameters(syntax, diagnostics);

CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody, diagnostics);

if (hasBody)
Expand All @@ -74,8 +71,6 @@ protected SourceOrdinaryMethodSymbolBase(
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: isExplicitInterfaceImplementation, diagnostics, location);
}

protected abstract ImmutableArray<TypeParameterSymbol> MakeTypeParameters(CSharpSyntaxNode node, BindingDiagnosticBag diagnostics);

#nullable enable
protected override void MethodChecks(BindingDiagnosticBag diagnostics)
{
Expand All @@ -89,7 +84,7 @@ protected override void MethodChecks(BindingDiagnosticBag diagnostics)
{
for (int i = 0; i < declaredConstraints.Length; i++)
{
var typeParameter = _typeParameters[i];
var typeParameter = this.TypeParameters[i];
ErrorCode report;

switch (declaredConstraints[i].Constraints & (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType | TypeParameterConstraintKind.Default))
Expand Down Expand Up @@ -171,11 +166,6 @@ private void CompleteAsyncMethodChecks(BindingDiagnosticBag diagnosticsOpt, Canc

protected abstract void CompleteAsyncMethodChecksBetweenStartAndFinish();

public override ImmutableArray<TypeParameterSymbol> TypeParameters
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
get { return _typeParameters; }
}

public abstract override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken));

public override string Name
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// 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.

#nullable disable
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

using System.Collections.Immutable;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
/// <summary>
/// Wrapper around type-parameter/constraints/constraint-kind info. We wrap this information (instead of inlining
/// directly within type/method symbols) as most types/methods are not generic. As such, all those non-generic
/// types can point at the singleton sentivel <see cref="Empty"/> value, and avoid two pointers of overhead.
Copy link
Member

@jcouv jcouv May 8, 2023

Choose a reason for hiding this comment

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

sentivel

typo: sentinel #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed!

/// </summary>
internal sealed class TypeParameterInfo
{
public ImmutableArray<TypeParameterSymbol> LazyTypeParameters;

/// <summary>
/// A collection of type parameter constraint types, populated when
/// constraint types for the first type parameter are requested.
/// </summary>
public ImmutableArray<ImmutableArray<TypeWithAnnotations>> LazyTypeParameterConstraintTypes;

/// <summary>
/// A collection of type parameter constraint kinds, populated when
/// constraint kinds for the first type parameter are requested.
/// </summary>
public ImmutableArray<TypeParameterConstraintKind> LazyTypeParameterConstraintKinds;

public static readonly TypeParameterInfo Empty = new TypeParameterInfo(
ImmutableArray<TypeParameterSymbol>.Empty, ImmutableArray<ImmutableArray<TypeWithAnnotations>>.Empty, ImmutableArray<TypeParameterConstraintKind>.Empty);

public TypeParameterInfo() : this(default, default, default)
{
}

private TypeParameterInfo(
ImmutableArray<TypeParameterSymbol> typeParameters,
ImmutableArray<ImmutableArray<TypeWithAnnotations>> typeParameterConstraintTypes,
ImmutableArray<TypeParameterConstraintKind> typeParameterConstraintKinds)
{
LazyTypeParameters = typeParameters;
LazyTypeParameterConstraintTypes = typeParameterConstraintTypes;
LazyTypeParameterConstraintKinds = typeParameterConstraintKinds;
}

public static TypeParameterInfo Create(ImmutableArray<TypeParameterSymbol> typeParameters)
{
// If we have no type parameters (common case), we can just point at the singleton empty instance.
return typeParameters.IsEmpty ? Empty : new TypeParameterInfo(typeParameters, default, default);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected SynthesizedRecordOrdinaryMethod(SourceMemberContainerTypeSymbol contai

internal sealed override LexicalSortKey GetLexicalSortKey() => LexicalSortKey.GetSynthesizedMemberKey(_memberOffset);

protected sealed override ImmutableArray<TypeParameterSymbol> MakeTypeParameters(CSharpSyntaxNode node, BindingDiagnosticBag diagnostics) => ImmutableArray<TypeParameterSymbol>.Empty;
public sealed override ImmutableArray<TypeParameterSymbol> TypeParameters => ImmutableArray<TypeParameterSymbol>.Empty;

public sealed override ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTypeParameterConstraintTypes() => ImmutableArray<ImmutableArray<TypeWithAnnotations>>.Empty;

Expand Down