Skip to content

Avoid O(n²) array growth in GenerateTypeMappings - #11127

Merged
jonathanpeppers merged 2 commits into
mainfrom
jonathanpeppers/dev-peppers-fix-typemappings-list
Apr 16, 2026
Merged

Avoid O(n²) array growth in GenerateTypeMappings#11127
jonathanpeppers merged 2 commits into
mainfrom
jonathanpeppers/dev-peppers-fix-typemappings-list

Conversation

@jonathanpeppers

@jonathanpeppersjonathanpeppers commented Apr 16, 2026

Copy link
Copy Markdown
Member

Description

GenerateTypeMappings.AddOutputTypeMaps() called Concat().ToArray() on GeneratedBinaryTypeMaps for each architecture iteration, copying the entire array each time -- O(n²) growth.

This replaces that pattern with a backing List<ITaskItem> field that items are added to directly, with a single .ToArray() call at the end of RunTask() to satisfy the MSBuild [Output] property type requirement.

jonathanpeppersand others added 2 commits April 16, 2026 10:03
Replace Concat().ToArray() per-arch iteration with a backing
List<ITaskItem> field that uses AddRange(), converting to array
once at the end of RunTask().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add items directly to the backing List<ITaskItem> field instead of
building a local list and calling AddRange().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings April 16, 2026 15:17

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves performance in the GenerateTypeMappings MSBuild task by eliminating repeated Concat().ToArray() calls that caused O(n²) copying as typemap outputs were accumulated across architectures.

Changes:

  • Replace per-architecture array concatenation with a single backing List<ITaskItem> for generated typemap outputs.
  • Emit the MSBuild [Output]ITaskItem[] once at the end of RunTask() via a single ToArray() call.

@jonathanpeppersjonathanpeppers left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 0 errors, 0 warnings, 1 suggestion:

  • 💡 Formatting: Minor style nit — new List<ITaskItem> ()[] for consistency (GenerateTypeMappings.cs:53)

CI Note: All public checks pass ✅, but the internal Xamarin.Android-PR pipeline is failing — may need a re-run.

👍 Clean fix. The old Concat().ToArray() pattern copied the entire array on every architecture iteration — classic O(n²) growth. Replacing it with a backing List<ITaskItem> and a single .ToArray() at the end is the right call.


Review generated by android-reviewer from review guidelines.

@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

There is only a WearOS emulator issue -- merging.

@jonathanpeppers
jonathanpeppers merged commit 23bc9df into mainApr 16, 2026
9 of 10 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers/dev-peppers-fix-typemappings-list branch April 16, 2026 19:50
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jonathanpeppers