Perf: Eliminate LINQ allocations in BoxAndWhiskerSeries.GenerateSegments#370
Merged
PaulAndersonS merged 2 commits intoMay 27, 2026
Merged
Conversation
Replace per-iteration LINQ .Where().ToArray() with a manual loop-based FilterNaNValues helper that avoids closure and enumerator allocations. Replace .Average() with a manual sum loop to eliminate LINQ overhead. Replace .Min()/.Max() on an already-sorted array with direct index access (yList[0] and yList[^1]). These changes reduce GC pressure in the hot path that runs once per data point during chart segment generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SaiyathAliFathima
requested changes
May 27, 2026
|
|
||
| var result = new double[count]; | ||
| int index = 0; | ||
| for (int i = 0; i < source.Count; i++) |
Collaborator
There was a problem hiding this comment.
Why does FilterNaNValues(IList source) use two for loops (one to count valid values, then one to copy them) so that double.IsNaN is checked twice for each element, instead of using a single-pass approach?
Contributor
There was a problem hiding this comment.
The two-pass version is intentional because this helper returns a double[] with exact length while avoiding LINQ/list allocations. In a true single-pass approach, we’d either use List<double> (extra object + ToArray copy) or allocate source.Count and then resize/copy. For this hot path, avoiding those extra allocations/copies was prioritized over the extra IsNaN check.
SaiyathAliFathima
approved these changes
May 27, 2026
Collaborator
SaiyathAliFathima
left a comment
SaiyathAliFathima
left a comment
There was a problem hiding this comment.
We have ensured the changes and working fine
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause of the Issue
The
GenerateSegmentsmethod inBoxAndWhiskerSeriesruns a loop over every data point, and each iteration performed several unnecessary LINQ allocations:.Where(x => !double.IsNaN(x)).ToArray()— allocates a closure, an intermediateIEnumerableenumerator, and a final array on every iteration.Average()— LINQ extension that enumerates the array again with overhead.Min()/.Max()— iterates the entire sorted array when the result is simplyyList[0]/yList[^1]Description of Change
FilterNaNValueshelper: A simple two-pass loop (count then copy) that avoids closure/enumerator allocations entirely.yList.Average()with manual sum loop: Eliminates LINQ overhead for a trivial arithmetic operation.yList.Min()/yList.Max()with direct index access: Since the array is already sorted byArray.Sort(yList), the min and max are at known positions.These changes reduce GC pressure in a hot path that executes once per data point during chart segment generation.
Issues Fixed
Performance improvement — no associated issue.
Screenshots
N/A — no visual changes, behavior is identical.