search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.16k stars 4.72k forks source link

ilc doesn't trim some code referenced only from trimmed methods #105806

Closed MrJul closed 2 months ago

MrJul commented 2 months ago

Description

In some cases, ilc keeps some code in the final executable, but the only code path accessing it has been trimmed.

Reproduction Steps

Clone https://github.com/MrJul/repro-ilc-not-trimming Run dotnet publish -r win-x64

This is a very stripped down version of code from Avalonia reproducing the problem. (Note that in this example, only one type is kept, but in Avalonia there are more.)

Expected behavior

Everything in LinuxLib is trimmed, only WindowsLib is referenced.

Actual behavior

X11RenderingMode from LinuxLib isn't trimmed, but all methods using it have been removed.

In the following screenshots, we can notice that X11RenderingMode is referenced from X11PlatformOptions's constructor, which has been trimmed away: sizoscope types sizoscope paths

Regression?

No.

Known Workarounds

No response

Configuration

Latest Microsoft.DotNet.ILCompiler from daily build, 9.0.0-rc.1.24381.5

Other information

For comparison, ILLink works fine in this scenario: no LinuxLib assembly is visible in the trimmed folder.

Note that with .NET 8.0.7, the whole call path (X11PlatformOptions, AvaloniaX11PlatformExtensions, etc.) is incorrectly kept by ilc.

With .NET 9.0.0-rc.1.24381.5, only X11RenderingMode is kept. I couldn't find the PR that fixed the above, but it's probably related to this issue.

dotnet-policy-service[bot] commented 2 months ago

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

MichalStrehovsky commented 2 months ago

The compiler has two phases: scanning phase that figures out what is reachable, including reachable from reflection, and a compilation phase that does actual codegen. Compilation phase could get rid of this (through inlining and dead code elimination), but the scanning phase didn't (scanning phase doesn't inline). The damage to whole program view couldn't be undone. We might be able to do limited inlining for this case.

MichalStrehovsky commented 2 months ago

(The reason this was generated was reflection analysis - Sizoscope just doesn't show that. Rename the .codegen.dgml file to .scan.dgml (replacing the original scan.dgml in the obj directory) and reopen the mstat in sizoscope - it will show the type was kept due to reflection.)

MrJul commented 2 months ago

Thank you for your answer and the tip to rename the dgml file, it helps better understanding the root causes!

I realize my stripped down example was maybe a bit too simple. In this specific case, I assume the reflection usage is because ilc thinks it's needed for the underlying array type? (Even though it should still be trimmed.)

In a real Avalonia app, UsePlatformDetect() keeps a lot of other types unrelated to reflection - for example unused implementations of used interfaces.

My main concern here is that ILLink is able to figure out that all of these types are unreachable and must completely removed. I would naively expect ILC to have similar capabilities. (In fact I've used PublishTrimmed without PublishAot several times in the past to ensure that conditionally unreferenced code is properly trimmed before publishing with AOT, as it's faster to have a look at trimmed assemblies with various decompilers. But that was a bad assumption.)

Don't worry, I understand that it's not at easy as sharing some code between the two applications :)

MichalStrehovsky commented 2 months ago

In this specific case, I assume the reflection usage is because ilc thinks it's needed for the underlying array type? (Even though it should still be trimmed.)

Specifically for enum types, the compiler has an assumption that if the type was touched in a significant way, it was reflected on. Making an array of the enum is "significant".

The general problem is that we didn't "inline" the OS detection API calls even though they return constant true/false and looked at the (eventually) dead branch while building the whole program view. The fix is not to look at this dead branch. ILLink does that, ILC doesn't (right now). This is in a similar bucket as https://github.com/dotnet/runtime/pull/88796#issuecomment-1635216001. Would be nice to fix. I guess this issue will finally force me to look into it.