Reduce CFG memory allocations #5402
Conversation
|
I'm not going to fix the coverage issue, as the constructor is private and currently only reachable through |
| internal static class PropertyInfoExtensions | ||
| { | ||
| public static T ReadCached<T>(this PropertyInfo property, object instance, ref T cache) where T : class => | ||
| cache ??= (T)property.GetValue(instance); |
There was a problem hiding this comment.
This will have the downside that, if a value is null, we will continue to ask for it, even if we requested it before. As long as this method proves better in practice, this is not a problem but we should be aware of it.
There was a problem hiding this comment.
Yes, I considered that downside. For current usage, the BasicBlock.BranchValue is the hottest candidate to be null. And I assume it's going to be called 2 times per instance (once through OperationsAndBranchValue that cases the result, once for branching in the future).
The rest should be called zero or once (OriginalOperation, ExceptionType, Parent), or not be null (Children, Language, SemanticModel).
| public ControlFlowConditionKind ConditionKind => ConditionKindProperty.ReadCached(instance, ref conditionKind); | ||
| public ControlFlowRegion EnclosingRegion => EnclosingRegionProperty.ReadCached(instance, ControlFlowRegion.Wrap, ref enclosingRegion); | ||
| public ControlFlowBranch FallThroughSuccessor => FallThroughSuccessorProperty.ReadCached(instance, ControlFlowBranch.Wrap, ref fallThroughSuccessor); | ||
| public bool IsReachable => IsReachableProperty.ReadCached<bool>(instance, ref isReachable); |
There was a problem hiding this comment.
<bool> is not needed
| public ControlFlowRegion EnclosingRegion => EnclosingRegionProperty.ReadCached(instance, ControlFlowRegion.Wrap, ref enclosingRegion); | ||
| public ControlFlowBranch FallThroughSuccessor => FallThroughSuccessorProperty.ReadCached(instance, ControlFlowBranch.Wrap, ref fallThroughSuccessor); | ||
| public bool IsReachable => IsReachableProperty.ReadCached<bool>(instance, ref isReachable); | ||
| public BasicBlockKind Kind => KindProperty.ReadCached<BasicBlockKind>(instance, ref kind); |
There was a problem hiding this comment.
<BasicBlockKind> is not needed
| public bool IsReachable => IsReachableProperty.ReadCached<bool>(instance, ref isReachable); | ||
| public BasicBlockKind Kind => KindProperty.ReadCached<BasicBlockKind>(instance, ref kind); | ||
| public ImmutableArray<IOperation> Operations => OperationsProperty.ReadCached(instance, ref operations); | ||
| public int Ordinal => OrdinalProperty.ReadCached<int>(instance, ref ordinal); |
There was a problem hiding this comment.
<int> is not needed
pavel-mikula-sonarsource
force-pushed
the
Pavel/ReduceMemory-CFG
branch
from
1ba7c41 to
1358666
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
SonarCloud Quality Gate failed. |
Fixes #5401