Skip to content

Conversation

@adamsitnik
Copy link
Member

Reverts #2941, we don't use reflection on purpose

@timcassell
Copy link
Collaborator

The reason for not using reflection in AOT is because of trimming afaict. Dynamic dependency prevents that trimming and makes the code gen simpler for all toolchains. All the tests passed. What is the reason to revert?

@adamsitnik adamsitnik merged commit 8a73c91 into master Dec 31, 2025
21 checks passed
@adamsitnik adamsitnik deleted the revert-2941-dynamicdependency branch December 31, 2025 18:24
@adamsitnik
Copy link
Member Author

The reason for not using reflection in AOT is because of trimming afaict. Dynamic dependency prevents that trimming and makes the code gen simpler for all toolchains. All the tests passed. What is the reason to revert?

The ability to benchmark Native AOT in the mode where it removes reflection entirely.

@timcassell
Copy link
Collaborator

I see. Do we need that for wasm AOT as well?

@timcassell timcassell added this to the v0.16.0 milestone Dec 31, 2025
@adamsitnik
Copy link
Member Author

I see. Do we need that for wasm AOT as well?

Most likely not, but I am not sure if we should have two different AOT code paths (increased complexity and likelihood of errors)

@timcassell
Copy link
Collaborator

It looks like reflection-free mode was removed in dotnet/runtime#109857 (.Net 10 I think). I guess we still need to care for .Net 6-9.

@timcassell
Copy link
Collaborator

I tried adding a test with .WithMsBuildArguments($"/p:IlcDisableReflection=true") and it immediately crashed with

MT140702267607176: TypeInitialization_Type_NoTypeAvailable
 ---> MT140702267603848: Arg_NullReferenceException
   at System.Reflection.Runtime.Assemblies.RuntimeAssemblyInfo.<>c.<.cctor>b__85_0(RuntimeAssemblyName) + 0x32
   at System.Collections.Concurrent.ConcurrentUnifierW`2.GetOrAdd(K) + 0x5e
   at System.Reflection.Runtime.Assemblies.RuntimeAssemblyInfo.GetRuntimeAssemblyIfExists(RuntimeAssemblyName) + 0x2a
   at System.Reflection.TypeNameParser.GetType(String, ReadOnlySpan`1, String) + 0x10b
   at BenchmarkDotNet.Portability.RuntimeInformation..cctor() + 0x41e
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0xc1
   Exception_EndOfInnerExceptionStack
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x158
   at System.Runtime.CompilerServices.ClassConstructorRunner.CheckStaticClassConstructionReturnNonGCStaticBase(StaticClassConstructionContext*, IntPtr) + 0xd
   at BenchmarkDotNet.Portability.RuntimeInformation.GetRuntimeVersion() + 0x75a
   at BenchmarkDotNet.Environments.BenchmarkEnvironmentInfo..ctor() + 0x23
   at BenchmarkDotNet.Autogenerated.Runnable_0.Run(IHost, String, RunMode) + 0xac
   at BenchmarkDotNet.Autogenerated.UniqueProgramName.AfterAssemblyLoadingAttached(String[]) + 0x3eb

Given that it looks like we never actually supported that mode (we certainly never tested it), and runtime removed it, how do you feel about un-reverting these changes @adamsitnik?

@timcassell timcassell mentioned this pull request Jan 1, 2026
@timcassell
Copy link
Collaborator

Alternatively I put up #2951. Which may be better anyway in case someone wants to benchmark ReadyToRun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants