Fold generic method bodies by default - #117411

Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged

Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom
MichalStrehovsky:foldgeneric

Conversation

@MichalStrehovsky

@MichalStrehovskyMichalStrehovsky commented Jul 8, 2025•
edited
Loading

Copy link
Copy Markdown
Member

Fixes#103951.

Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).

Cc @dotnet/ilc-contrib

@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service

dotnet-policy-serviceBot commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

@MichalStrehovskyMichalStrehovsky mentioned this pull request Jul 8, 2025
Closed
@github-actionsgithub-actionsBot mentioned this pull request Jul 8, 2025
Closed
@jkotas

jkotas commented Jul 8, 2025

Copy link
Copy Markdown
Member

They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).

Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?

@MichalStrehovsky

MichalStrehovsky commented Jul 8, 2025

Copy link
Copy Markdown
MemberAuthor

They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).

Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?

I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.

@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 8, 2025 08:35
CopilotAI review requested due to automatic review settings July 8, 2025 08:35
@MichalStrehovsky

MichalStrehovsky commented Jul 8, 2025

Copy link
Copy Markdown
MemberAuthor

Size statistics

Pull request #117411

ProjectSize beforeSize afterDifference
TodosApi-linux 25538640 24666192 -872448
TodosApi-windows 22906368 22239232 -667136
avalonia.app-linux 19995056 19499440 -495616
avalonia.app-windows 17649664 17309696 -339968
hello-linux 1369040 1364944 -4096
hello-minimal-linux 1086248 1086248 0
hello-minimal-windows 837120 837120 0
hello-windows 1092608 1088512 -4096
kestrel-minimal-linux 5205784 5205784 0
kestrel-minimal-windows 4750848 4750848 0
reflection-linux 1992752 1964080 -28672
reflection-windows 1695232 1673728 -21504
webapiaot-linux 9866600 9653608 -212992
webapiaot-windows 9032704 8862720 -169984
winrt-component-full-windows 5720064 5594112 -125952
winrt-component-minimal-windows 1701888 1701888 0

CopilotAI reviewed Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.

  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.

Reviewed Changes

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

Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103

  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102

  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104

  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka reviewed Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion mentioned this pull request Jul 8, 2025
Open
This was referenced Jul 8, 2025
Open
Open
Closed
@MichalStrehovsky

MichalStrehovsky commented Jul 10, 2025

Copy link
Copy Markdown
MemberAuthor

@sbomer is this something you'd be comfortable reviewing?

@huoyaoyuanhuoyaoyuan mentioned this pull request Jul 12, 2025
Closed
sbomer
sbomer approved these changes Jul 17, 2025
View reviewed changes

@sbomersbomer 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.

Did you consider making the defaults dependent on DebuggerSupport?

Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky

MichalStrehovsky commented Jul 18, 2025

Copy link
Copy Markdown
MemberAuthor

Did you consider making the defaults dependent on DebuggerSupport?

I didn't think about it but gave it a thought now - here's my thinking:

  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.

So I'd lean towards not tying it to that.

@MichalStrehovsky
MichalStrehovsky merged commit 0621e64 into dotnet:mainJul 18, 2025
93 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the foldgeneric branch July 18, 2025 08:21
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 17, 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
@sbomersbomersbomer approved these changes
+1 more reviewer
@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default

5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Fold generic method bodies by default - #117411
Merged
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Jul 18, 2025
Merged
Fold generic method bodies by default#117411
MichalStrehovsky merged 5 commits into
dotnet:mainfrom 
MichalStrehovsky:foldgeneric
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Jul 8, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Fixes#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Fold generic method bodies by default
09e4f37
Fixesdotnet#103951.
Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
@MichalStrehovsky
temp change to run rt-sz
55811fd
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 8, 2025

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas

See info in area-owners.md if you want to be subscribed.
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Jul 8, 2025
 Closed

@github-actionsgithub-actionsBot
mentioned this pull request
Jul 8, 2025

Closed

@jkotas
jkotas
commented
Jul 8, 2025

Copy link


Copy Markdown

Member
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).
Do you think it would be possible to rename the symbol for folded copy such that the symbol name does not include the instantiation that happened to win?
I think it would make things quite complicated in general. We don't have facilities to change our mind about how symbols are named. We'd also need to invent a scheme for new stable names.
@MichalStrehovsky
MichalStrehovsky
marked this pull request as ready for review
July 8, 2025 08:35
CopilotAI
review requested due to automatic review settings
July 8, 2025 08:35
@MichalStrehovsky
MichalStrehovsky
commented
Jul 8, 2025

Copy link


Copy Markdown

MemberAuthor
Size statistics
Pull request #117411
ProjectSize beforeSize afterDifference
TodosApi-linux 25538640  24666192 -872448
TodosApi-windows 22906368  22239232 -667136
avalonia.app-linux 19995056  19499440 -495616
avalonia.app-windows 17649664  17309696 -339968
hello-linux 1369040  1364944 -4096
hello-minimal-linux 1086248  1086248 0
hello-minimal-windows 837120  837120 0
hello-windows 1092608  1088512 -4096
kestrel-minimal-linux 5205784  5205784 0
kestrel-minimal-windows 4750848  4750848 0
reflection-linux 1992752  1964080 -28672
reflection-windows 1695232  1673728 -21504
webapiaot-linux 9866600  9653608 -212992
webapiaot-windows 9032704  8862720 -169984
winrt-component-full-windows 5720064  5594112 -125952
winrt-component-minimal-windows 1701888  1701888 0
CopilotAI
reviewed
Jul 8, 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 replaces the boolean “fold identical method bodies” flag with a three-way MethodBodyFoldingMode enum (None, Generic, All), defaults generic folding by build integration, and wires the new mode through the compiler and MSBuild targets.
  • Swapped UseMethodBodyFolding(bool) for UseMethodBodyFolding(MethodBodyFoldingMode) in the CLI, builder, and RyuJit compilation paths.
  • Introduced MethodBodyFoldingMode enum and revamped ObjectDataInterner to only fold based on generics when requested.
  • Updated MSBuild targets to compute a default folding mode and emit the new --methodbodyfolding:<mode> argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler/Program.csParse string option into new MethodBodyFoldingMode
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.csChange CLI option type from bool to string and update description
src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.csSwitch interner creation to use new enum
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.csAdd _genericsOnly flag, CanFold, and comparer support
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.csUse CanFold instead of IsNull
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.csChange builder API and add enum definition
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targetsCompute and pass <mode> to --methodbodyfolding
Comments suppressed due to low confidence (3)
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:103
  • The description is missing a closing parenthesis; change to e.g. "Fold identical method bodies (one of: none, generic, all)".
 new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:102
  • Add an AddValidator to restrict the option to the valid values (none, generic, all) and produce a clear error when parsing fails.
 public Option<string> MethodBodyFolding { get; } =

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:104
  • No tests appear to validate the new MethodBodyFolding modes; please add unit tests for parsing and default behavior to ensure coverage.
 public Option<string[]> InitAssemblies { get; } =

Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
MichalPetryka
MichalPetryka
reviewed
Jul 8, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
@SingleAccretionSingleAccretion
mentioned this pull request
Jul 8, 2025

Open


This was referenced Jul 8, 2025

Open


Open


Closed

@MichalStrehovsky
MichalStrehovsky
commented
Jul 10, 2025

Copy link


Copy Markdown

MemberAuthor
@sbomer is this something you'd be comfortable reviewing?
@huoyaoyuanhuoyaoyuan
mentioned this pull request
Jul 12, 2025

Closed

sbomer
sbomer
approved these changes
Jul 17, 2025
View reviewed changes
@sbomersbomer
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.

Did you consider making the defaults dependent on DebuggerSupport?
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs
@MichalStrehovsky
MichalStrehovsky
commented
Jul 18, 2025

Copy link


Copy Markdown

MemberAuthor
Did you consider making the defaults dependent on DebuggerSupport?
I didn't think about it but gave it a thought now - here's my thinking:
  • DebuggerSupport disables debugger support. For example, it controls whether we leave the breadcrumbs for the SOS extension to work. The SOS extension doesn't work at all with DebuggerSupport=false. Debugging is much harder.
  • Method folding somewhat messes up the callstack, but not to the extent that it would be very difficult to reason about. It's more to the tune of how inlining and tail calls mess up callstack.
So I'd lean towards not tying it to that.
@MichalStrehovsky
MichalStrehovsky
merged commit 0621e64
into
dotnet:mainJul 18, 2025

93 checks passed

@MichalStrehovsky
MichalStrehovsky
deleted the

foldgeneric

branch
July 18, 2025 08:21
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 17, 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
@sbomersbomersbomer approved these changes

+1 more reviewer

@MichalPetrykaMichalPetrykaMichalPetryka left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Investigate folding identical methods with genericness by default


5 participants

@MichalStrehovsky@jkotas@sbomer@MichalPetryka