Avoid intermediary array allocation in incremental generator#62100
Avoid intermediary array allocation in incremental generator#62100CyrusNajmabadi merged 6 commits intodotnet:mainfrom
Conversation
| public ImmutableArray<NodeStateEntry<T>> Batch() | ||
| public IEnumerable<NodeStateEntry<T>> Batch() | ||
| { | ||
| var sourceBuilder = ArrayBuilder<NodeStateEntry<T>>.GetInstance(); |
There was a problem hiding this comment.
this the array we're avoiding creating.
| } | ||
|
|
||
| public ImmutableArray<NodeStateEntry<T>> Batch() | ||
| public IEnumerable<NodeStateEntry<T>> Batch() |
There was a problem hiding this comment.
sure. let me try that.
|
@cston much nicer. thanks! |
|
|
||
| foreach (var entry in sourceTable) | ||
| { | ||
| if (entry.State != EntryState.Removed) |
There was a problem hiding this comment.
no. that was only used for adding information about steps. but we always do that in the line after this if-check.
| if (entry.State != EntryState.Removed) | ||
| sourceValuesBuilder.Add(entry.Item); | ||
|
|
||
| sourceInputsBuilder?.Add((entry.Step!, entry.OutputIndex)); |
There was a problem hiding this comment.
no. we always want to add the information about the steps including for a 'remove'. i can add comments.
| if (entry.State != EntryState.Removed || HasTrackedSteps) | ||
| { | ||
| sourceBuilder.Add(entry); | ||
| } |
There was a problem hiding this comment.
@cston the HasTrackedSteps was to make sure that we retruend this entry from the iterator so we could associated it with the perf data we're collecting. On the receiver side though we then filtered this out from the actual entry-list we were creating, but we used it for the step-list being built.
|
|
||
| var batchedSourceEntries = sourceTable.Batch(); | ||
| var sourceValues = batchedSourceEntries.SelectAsArray(sourceEntry => sourceEntry.State != EntryState.Removed, sourceEntry => sourceEntry.Item); | ||
| var sourceInputs = newTable.TrackIncrementalSteps ? batchedSourceEntries.SelectAsArray(sourceEntry => (sourceEntry.Step!, sourceEntry.OutputIndex)) : default; |
There was a problem hiding this comment.
@cston here you can see the impact. batchedSourceEntries.SelectAsArray(sourceEntry => sourceEntry.State != EntryState.Removed removes the removed items that were included because we were tracking steps. but batchedSourceEntries.SelectAsArray(sourceEntry => (sourceEntry.Step!, sourceEntry.OutputIndex) keeps everything. The logic we have below preserve that. we enumerate everything, do not include the removed-items from the values-builder, but keeps the steps fro the for the steps-builder.
This drops us from 3GB of alloc to 2.3 GB (a good 25% reduction) during incremental generation.
Specifically it removes this array:
Before:
After:
