Skip to content

Perf: Hoist ToList() out of loops in CartesianChartArea.UpdateSBS and GetTotalWidth#373

Closed
PaulAndersonS wants to merge 3 commits into
mainfrom
paulandersons/perf-optimize-chart-bindable-layout
Closed

Perf: Hoist ToList() out of loops in CartesianChartArea.UpdateSBS and GetTotalWidth#373
PaulAndersonS wants to merge 3 commits into
mainfrom
paulandersons/perf-optimize-chart-bindable-layout

Conversation

@PaulAndersonS

Copy link
Copy Markdown
Collaborator

Root Cause of the Issue

In CartesianChartArea.cs, both UpdateSBS() and GetTotalWidth() called SideBySideSeriesPosition.Values.ToList()[i] inside a for-loop. This creates a brand new List<List<CartesianSeries>> on every iteration just to index into it — resulting in O(n) unnecessary heap allocations where only 1 is needed.

For charts with many series groups, this wastes memory and puts pressure on the garbage collector during layout calculations.

Description of Change

Hoisted the .Values.ToList() call before the loop in both methods, materializing the dictionary values once and reusing the cached list for indexed access.

Before (called N times):

for (int i = 0; i < SideBySideSeriesPosition.Count; i++)
{
    var seriesGroup = SideBySideSeriesPosition.Values.ToList()[i]; // allocates every iteration
}

After (called once):

var seriesGroups = SideBySideSeriesPosition.Values.ToList(); // single allocation
for (int i = 0; i < seriesGroups.Count; i++)
{
    var seriesGroup = seriesGroups[i];
}

Also added a unit test UpdateSBS_WithMultipleSeriesGroups_ShouldComputeSbsInfo to verify correct behavior with multiple series groups.

Issues Fixed

Performance improvement — no specific issue.

Screenshots

N/A — internal performance optimization with no visual change.

In UpdateSBS() and GetTotalWidth(), SideBySideSeriesPosition.Values.ToList()
was called inside a for-loop, creating a new List<> allocation on every
iteration. This is O(n) allocations where only 1 is needed.

Hoisted the ToList() call before both loops so the dictionary values are
materialized once and reused.

Also adds a unit test for UpdateSBS with multiple series groups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@VimalaThirumalaikumar VimalaThirumalaikumar left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes fine

@VimalaThirumalaikumar

Copy link
Copy Markdown

@PaulAndersonS sir, the source level PR changes was already moved in https://github.com/syncfusion/maui-toolkit/pull/384/changes - because of it, when rebased the source level file changes was 0.

As discussed with @SaiyathAliFathima, for performance improvement changes - ignoring unit test case inclusion, hence please consider closing the PR

@PaulAndersonS

Copy link
Copy Markdown
Collaborator Author

Duplicate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants