[NativeAOT] System.Linq.Expressions size regression

[eerhardt]

Description

The "Stage2" / "TodosAPI" ASP.NET benchmark app's NativeAOT size has regressed in recent builds.

As you can see, the size increased from 31 MB to 41 MB.

Looking at the mstat diffs, I see most of the size increase coming from System.Linq.Expressions. This leads me to believe this was regressed by #89308.

Configuration

All

Regression?

Yes

Data

The before and after mstat files can be found in:
MStatFiles.zip

Analysis

Diffing the attached mstat files in sizoscope shows:

cc @MichalStrehovsky @ivanpovazan

[ghost]

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

Issue Details

---

Description

The "Stage2" / "TodosAPI" ASP.NET benchmark app's NativeAOT size has regressed in recent builds.

As you can see, the size increased from 31 MB to 41 MB.

Looking at the mstat diffs, I see most of the size increase coming from System.Linq.Expressions. This leds me to believe this was regressed by #89308.

Configuration

All

Regression?

Yes

Data

The before and after mstat files can be found in:
MStatFiles.zip

Analysis

Diffing the attached mstat files in sizoscope shows:

cc @MichalStrehovsky @ivanpovazan

Author:	eerhardt
Assignees:	-
Labels:	tenet-performance, area-NativeAOT-coreclr
Milestone:	-

[MichalStrehovsky]

Looks like we no longer trim the Ref.Emit backend or the CanCreateArbitraryDelegates code path. It likely is from #89308. I think we'll need to undo removal of the other two switches and only leave the CanEmitObjectArrayDelegate change. (The two switches can/should stay as --feature).

[ivanpovazan]

That is a bit unexpected as both:

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/LambdaExpression.cs

Line 29 in bcb608e

public static bool CanCompileToIL => RuntimeFeature.IsDynamicCodeSupported;

and

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs

Line 19 in bcb608e

private static bool CanCreateArbitraryDelegates => RuntimeFeature.IsDynamicCodeSupported;

are guarded by IsDynamicCodeSupported which should be set to false in NativeAOT scenarios:

runtime/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets

Line 41 in bcb608e

<DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == ''">false</DynamicCodeSupport>

I will try to repro and revert if needed.

[MichalStrehovsky]

Dead branch removal rules are undocumented and unreliable. One needs to basically always re-test if this is shuffled. I think the problem here is that if we're in a situation where there is if (Something()) { ... } and Something() becomes obviously constant only after inspecting that method body, the branch will not be removed by NativeAOT. It will be removed by ILLinker if it runs before NativeAOT because ILLinker does propagate across methods but we don't run with ILLinker outside AppleMobile.

[eerhardt]

I was able to verify the regression happened between 5b4bbda...d389ab9.

To repro:

dotnet publish the app at https://github.com/aspnet/Benchmarks/tree/main/src/BenchmarksApps/TodosApi. You can use different official builds of the runtime by changing the .csproj:

<ItemGroup>
  <FrameworkReference Update="Microsoft.NETCore.App"
                      RuntimeFrameworkVersion="8.0.0-rc.1.23374.1" />
  <PackageReference Include="Microsoft.Dotnet.ILCompiler"
                    Version="8.0.0-rc.1.23374.1" />
</ItemGroup>

With that version, the size on my win-x64 machine is 27.4 MB. Note that version is from commit 5b4bbda.

<ItemGroup>
  <FrameworkReference Update="Microsoft.NETCore.App"
                      RuntimeFrameworkVersion="8.0.0-rc.1.23375.8" />
  <PackageReference Include="Microsoft.Dotnet.ILCompiler"
                    Version="8.0.0-rc.1.23375.8" />
</ItemGroup>

With that version, the size on my win-x64 machine is 34.4 MB. Note that version is from commit d389ab9.

[ivanpovazan]

I have verified that the regression is introduced with: #89308
I will try to get a fix for this soon - targeting rc1.
If it is urgent to get steady perf reports, I can revert the change and apply a proper fix afterwards.