Skip to content

[clr-interp] Fix performance of compiling huge methods - #119514

Merged
davidwrighton merged 2 commits into
dotnet:mainfrom
davidwrighton:fix_huge_method_compile_time
Sep 11, 2025
Merged

[clr-interp] Fix performance of compiling huge methods#119514
davidwrighton merged 2 commits into
dotnet:mainfrom
davidwrighton:fix_huge_method_compile_time

Conversation

@davidwrighton

@davidwrightondavidwrighton commented Sep 9, 2025•
edited
Loading

Copy link
Copy Markdown
Member
  • Remove O(n) algorithm in InterpCompiler::EmitCodeIns which was debug only, replace with O(1) algorithm
  • The new conservative range computuation code also has an O(N^2) algorithm for ranges, and this is replaced with an O(n*logN) algorithm with better constant factors (Binary search in an array instead of linear scan of a linked list)
  • Adjust apis on TArray so that the RemoveAt and Remove functions don't re-order the array. Change the names of the existing apis to have a Unordered suffix, and add more typical implementations of RemoveAt and InsertAt

@davidwrighton
[clr-interp] Fix performance of compiling huge methods
748fddb
- Remove O(n) algorithm in InterpCompiler::EmitCodeIns which was debug only, replace with O(1) algorithm
- The new conservative range computuation code also has an O(N^2) algorithm for ranges, and this is replaced with an O(n*logN) algorithm with better constant factors (Binary search in an array instead of linear scan of a linked list)
- Adjust apis on TArray so that the RemoveAt and Remove functions don't re-order the array. Change the names of the existing apis to have a _ArrayIsBag suffix, and add more typical implementations of RemoveAt and InsertAt
CopilotAI review requested due to automatic review settings September 9, 2025 23:45
@github-actionsgithub-actionsBot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 9, 2025
@davidwrightondavidwrighton added area-CodeGen-Interpreter-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 9, 2025
CopilotAI reviewed Sep 9, 2025
View reviewed changes

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 the performance of compiling large methods in the CLR interpreter by addressing O(N²) and O(N) algorithmic bottlenecks in critical compilation paths.

Key changes:

  • Replaces O(N) debug-only IL offset validation with O(1) hash table lookup using a side table
  • Refactors conservative GC range computation from O(N²) linked list operations to O(N log N) binary search with array-based storage
  • Enhances TArray class with proper order-preserving RemoveAt/InsertAt methods while renaming existing bag-style operations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

FileDescription
src/coreclr/interpreter/datastructs.hAdds Set method, order-preserving RemoveAt/InsertAt, and renames bag-style operations
src/coreclr/interpreter/compileropt.cppUpdates method calls to use renamed bag-style array operations
src/coreclr/interpreter/compiler.hAdds debug-only side table field and fixes operator formatting
src/coreclr/interpreter/compiler.cppImplements O(1) IL offset validation and O(N log N) conservative range computation

Comment threadsrc/coreclr/interpreter/datastructs.h Outdated
Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
@dotnet-policy-service

dotnet-policy-serviceBot commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@davidwrighton
davidwrighton marked this pull request as draft September 9, 2025 23:48
@davidwrighton

davidwrighton commented Sep 9, 2025

Copy link
Copy Markdown
MemberAuthor

Oops, the allocation changes aren't safe. Sigh. I'll have to think on that.

kg
kg reviewed Sep 10, 2025
View reviewed changes
Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
kg
kg reviewed Sep 10, 2025
View reviewed changes
Comment threadsrc/coreclr/interpreter/datastructs.h Outdated
@davidwrighton
Make TArray use an allocator, and implement one based on the MemPool …
2470f8f
…and one based on malloc/free
- Fix the TArray InsertAt function
@davidwrighton
davidwrighton marked this pull request as ready for review September 10, 2025 21:38
@build-analysisbuild-analysisBot mentioned this pull request Sep 10, 2025
Open
janvorli
janvorli reviewed Sep 11, 2025
View reviewed changes
Comment threadsrc/coreclr/interpreter/datastructs.h
Comment threadsrc/coreclr/interpreter/compiler.cpp
Comment threadsrc/coreclr/interpreter/compiler.cpp
janvorli
janvorli approved these changes Sep 11, 2025
View reviewed changes

@janvorlijanvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM modulo the unused local comment, thank you!

@davidwrighton
davidwrighton merged commit 8fd0afd into dotnet:mainSep 11, 2025
95 of 97 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Oct 12, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@BrzVladBrzVladAwaiting requested review from BrzVladBrzVlad is a code owner
@kgkgAwaiting requested review from kg

Assignees

@davidwrightondavidwrighton

Labels

area-CodeGen-Interpreter-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@davidwrighton@kg@janvorli