Skip to content

Commit

Permalink
Use a singleton sentinel for empty type-parameters for non-generic na…
Browse files Browse the repository at this point in the history
…med-types and methods. (#68131)

* Use a singleton for empty type-parameters for named types

* move type parameters down

* Optimize methods as well

* Always create indirection

* Move type

* Apply suggestions from code review

* speeling

* nrt

* Inline

* Simplify

* Change to abstract override
  • Loading branch information
CyrusNajmabadi authored May 11, 2023
1 parent b404fbe commit 236149d
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 66 deletions.
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,13 @@ private SourceOrdinaryMethodSymbol(
{
Debug.Assert(diagnostics.DiagnosticBag is object);

// Compute the type parameters. If empty (the common case), directly point at the singleton to reduce the
// amount of pointers-to-arrays this type needs to store.
var typeParameters = MakeTypeParameters(syntax, diagnostics);
_typeParameterInfo = typeParameters.IsEmpty
? TypeParameterInfo.Empty
: new TypeParameterInfo { LazyTypeParameters = typeParameters };

_explicitInterfaceType = explicitInterfaceType;

bool hasBlockBody = syntax.Body != null;
Expand All @@ -105,17 +99,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 +297,7 @@ protected override void CompleteAsyncMethodChecksBetweenStartAndFinish()

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

Expand All @@ -324,19 +313,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 +340,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 +523,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;
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,10 +166,7 @@ private void CompleteAsyncMethodChecks(BindingDiagnosticBag diagnosticsOpt, Canc

protected abstract void CompleteAsyncMethodChecksBetweenStartAndFinish();

public override ImmutableArray<TypeParameterSymbol> TypeParameters
{
get { return _typeParameters; }
}
public abstract override ImmutableArray<TypeParameterSymbol> TypeParameters { get; }

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

Expand Down
37 changes: 37 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/TypeParameterInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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.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 sentinel <see cref="Empty"/> value, and avoid two pointers of overhead.
/// </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
{
LazyTypeParameters = ImmutableArray<TypeParameterSymbol>.Empty,
LazyTypeParameterConstraintTypes = ImmutableArray<ImmutableArray<TypeWithAnnotations>>.Empty,
LazyTypeParameterConstraintKinds = ImmutableArray<TypeParameterConstraintKind>.Empty,
};
}
}
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

0 comments on commit 236149d

Please sign in to comment.