Reduce stack frame size of CheckConstraints #43094
Conversation
cston
force-pushed
the
42361
branch
4 times, most recently
from
75e8be2 to
db08f2b
Compare
| type.VisitType(s_checkConstraintsSingleTypeFunc, new CheckConstraintsArgs(compilation, conversions, includeNullability, location, diagnostics)); | ||
| } | ||
|
|
||
| private readonly struct CheckConstraintsArgs |
There was a problem hiding this comment.
CheckConstraintsArgs [](start = 32, length = 20)
Moving this structure is going to create unnecessary merge conflicts with feature branches, please move it back #Closed
| if (type.Kind == SymbolKind.NamedType) | ||
| { | ||
| ((NamedTypeSymbol)type).CheckConstraints(args.CurrentCompilation, args.Conversions, args.IncludeNullability, args.Location, args.Diagnostics); | ||
| ((NamedTypeSymbol)type).CheckConstraints(args); |
There was a problem hiding this comment.
args [](start = 57, length = 4)
Would it make sense to pass it by ref here as well? #ByDesign
There was a problem hiding this comment.
The argument is passed by reference - the CheckConstraints() parameter is in.
In reply to: 405099677 [](ancestors = 405099677)
There was a problem hiding this comment.
nit: consider using an explicit 'in' where possible for clarity.
src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs
Show resolved
Hide resolved
| var diagnosticsBuilder = ArrayBuilder<TypeParameterDiagnosticInfo>.GetInstance(); | ||
| ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder = null; | ||
| var result = !typeSyntax.HasErrors && CheckTypeConstraints(type, conversions, includeNullability, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: diagnosticsBuilder, ref useSiteDiagnosticsBuilder); | ||
| var result = !typeSyntax.HasErrors && CheckTypeConstraints(type, conversions, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: includeNullability ? diagnosticsBuilder : null, ref useSiteDiagnosticsBuilder); |
There was a problem hiding this comment.
nit: line is 244 characters, consider wrapping new lines for the arguments
There was a problem hiding this comment.
I'll leave these long lines as-is since the issues were not introduced in this change.
In reply to: 405767579 [](ancestors = 405767579)
| var diagnosticsBuilder = ArrayBuilder<TypeParameterDiagnosticInfo>.GetInstance(); | ||
| ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder = null; | ||
| var result = CheckTypeConstraints(type, conversions, includeNullability, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: diagnosticsBuilder, ref useSiteDiagnosticsBuilder); | ||
| var result = CheckTypeConstraints(type, args.Conversions, args.CurrentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: args.IncludeNullability ? diagnosticsBuilder : null, ref useSiteDiagnosticsBuilder); |
| var typeParameter = typeParameters[i]; | ||
|
|
||
| if (!CheckConstraints(containingSymbol, conversions, includeNullability, substitution, typeParameter, typeArgument, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt, ref useSiteDiagnosticsBuilder, | ||
| if (!CheckConstraints(containingSymbol, conversions, substitution, typeParameter, typeArgument, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt, ref useSiteDiagnosticsBuilder, |
| { | ||
| int n = (ExecutionConditionUtil.Architecture, ExecutionConditionUtil.Configuration) switch | ||
| { | ||
| (ExecutionArchitecture.x86, ExecutionConfiguration.Debug) => 420, |
There was a problem hiding this comment.
Is this change fixing a regression in our ability to handle complex constraint graphs? Or improving our ability past where it has historically been?
There was a problem hiding this comment.
The change improves the ability to handle complex constraint graphs, although 15.9 uses less stack space still.
In reply to: 405771709 [](ancestors = 405771709)
3 participants
Reduce the size of the stack frames in
ConstraintsHelper.CheckConstraints()overloads for nested constraint types.For the cycle of calls from
SourceTypeParameterSymbolBase.GetBounds(), reduces the overhead by:Fixes #42361