Pack bits in SourceOrdinaryMethodSymbol into an existing bitflag structure we have for all source methods #68132
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
| { | ||
| get { return (_flags & IsVarArgBit) != 0; } | ||
| set { ThreadSafeFlagOperations.Set(ref _flags, value ? IsVarArgBit : 0); } | ||
| } |
There was a problem hiding this comment.
note: IsVarArg is currently lazily computed. But it woudl be trivial to compute up front in SourceOrdinaryMethod's constructor and just not have to deal with this. But i kept the logic as before.
| private readonly bool _isExpressionBodied; | ||
| private readonly bool _hasAnyBody; | ||
| private readonly RefKind _refKind; | ||
| private bool _lazyIsVararg; |
There was a problem hiding this comment.
this was an additional 32bits per method
| @@ -12,7 +12,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols | |||
| { | |||
| internal sealed class SourceConstructorSymbol : SourceConstructorSymbolBase | |||
| { | |||
| private readonly bool _isExpressionBodied; | |||
| private readonly bool _hasThisInitializer; | |||
There was a problem hiding this comment.
todo: consider adding a flag for this. would take away an unnecessary 32bit in a constructor.
| @@ -23,7 +23,6 @@ internal class SourcePropertyAccessorSymbol : SourceMemberMethodSymbol | |||
| private ImmutableArray<MethodSymbol> _lazyExplicitInterfaceImplementations; | |||
| private string _lazyName; | |||
| private readonly bool _isAutoPropertyAccessor; | |||
| private readonly bool _isExpressionBodied; | |||
| private readonly bool _usesInit; | |||
There was a problem hiding this comment.
todo: consider bits for these two fields. That would be another 32 bits saved.
|
@jaredpar ptal. |
| { | ||
| // Events cannot be expression-bodied | ||
| get { return false; } | ||
| } |
There was a problem hiding this comment.
this is just wrong (but was overridden, so it wasn't a problem). Event accessors can def be expression-bodied.
src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs
Outdated
Show resolved
Hide resolved
ghost
added this to the 
| public bool IsVarArg | ||
| { | ||
| get { return (_flags & IsVarArgBit) != 0; } | ||
| set { ThreadSafeFlagOperations.Set(ref _flags, value ? IsVarArgBit : 0); } |
There was a problem hiding this comment.
set { ThreadSafeFlagOperations.Set(ref _flags, value ? IsVarArgBit : 0); }
This doesn't look like the right thing to do. If the value is lazy calculated, then there should be a flag indicating that it has been calculated and an attempt to get the value should trigger the calculation. Alternatively, if the value is easy to calculate from the syntax, we should do that and store the final value from the start, eliminating the ability to adjust the value later.
There was a problem hiding this comment.
Sure. I can move this to be computed up front.
…ing bitflag structure we have for all source methods (dotnet#68132)"" This reverts commit d51ec38.
…cture we have for all source methods (dotnet#68132) * In progresS * Utilize * initialize once * Save space in other methods * reorder * reorder * Move down * Reorder * Simplify * Add docs * move up
…cture we have for all source methods (#68158) * Pack bits in SourceOrdinaryMethodSymbol into an existing bitflag structure we have for all source methods (#68132) * In progresS * Utilize * initialize once * Save space in other methods * reorder * reorder * Move down * Reorder * Simplify * Add docs * move up * Compute IsVarArg up front * Remove unused usings * Remove * Remove passing of flag * move up and make non-virtual * "move up and make sealed" * move up and make sealed * use properties instead of flags * Make parameters non-optional * Pass all args * move more down * move more down * Pass all args * move more down * move more down * move more down * move more down * Name consistently * Set flag consistently * Rename * Fix usage of hasBody where it means hasBlockBody * Change to true * Change to true * Fix vararg check * Change to true * NRT * Pass values explicitly * Add assert * Flip depending on if something is abstract or not. * Update check * Fix ocmment * Fix ocmment * Update src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs
Drop 0.8% of memory from:
to