Improve performance of IsCollectible reflection properties

[jkotas]

These properties are often used for non-collectible fast paths. It is important that they are cheap. Converted these properties to FCalls to reduce overhead.

Fixes #119499

[Copilot]

Pull Request Overview

This PR improves the performance of IsCollectible reflection properties by converting them from QCalls to FCalls to reduce overhead. The change affects RuntimeType, RuntimeMethodInfo, and RuntimeAssembly IsCollectible properties, which are frequently used in non-collectible fast paths where performance is critical.

Key changes:

• Converted QCall implementations to FCall implementations for better performance
• Added new IsCollectible methods to TypeHandle and MethodDesc classes
• Updated method signatures and calling conventions across the reflection API

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/coreclr/vm/typehandle.h	Added IsCollectible method declaration to TypeHandle class
src/coreclr/vm/typehandle.cpp	Implemented IsCollectible method and updated GetLoaderAllocator contract
src/coreclr/vm/runtimehandles.h	Added FCall declarations for IsCollectible methods and removed QCall declarations
src/coreclr/vm/runtimehandles.cpp	Converted IsCollectible implementations from QCall to FCall pattern
src/coreclr/vm/qcallentrypoints.cpp	Removed QCall entry points for IsCollectible methods
src/coreclr/vm/method.hpp	Added IsCollectible method declaration to MethodDesc class
src/coreclr/vm/method.cpp	Implemented MethodDesc::IsCollectible and updated GetLoaderAllocator contract
src/coreclr/vm/ecalllist.h	Added FCall entries for IsCollectible methods
src/coreclr/vm/assemblynative.hpp	Added FCall declaration and removed QCall declaration for Assembly IsCollectible
src/coreclr/vm/assemblynative.cpp	Converted Assembly IsCollectible from QCall to FCall implementation
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs	Simplified IsCollectible property to use new FCall
src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs	Updated method signatures from QCall to FCall pattern
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs	Updated IsCollectible implementation with GC.KeepAlive for safety
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs	Updated IsCollectible implementation with GC.KeepAlive for safety

[jkotas]

@EgorBot -intel

using BenchmarkDotNet.Attributes;

public class Bench
{
    [Benchmark]
    public bool IsCollectible() => typeof(object).IsCollectible;
}

[jkotas]

@EgorBot -intel

using BenchmarkDotNet.Attributes;

public class Bench
{
    [Benchmark]
    public bool IsCollectible() => typeof(object).IsCollectible;
}

[ericstj]

I ran the benchmark I was using to investigate this issue against this branch. Good is before regression, bad is after, cache is my PR which cached IsCollectible, and fcall is this PR.

Method	Job	Toolchain	typeToConvert	Mean	Error	StdDev	Ratio
GetConverter	Job-USXRTJ	\bad\shared\Microsoft.NETCore.App\10.0.0\corerun.exe	Int32	101.88 ns	0.428 ns	0.400 ns	1.00
GetConverter	Job-AHBPSN	\cache\shared\Microsoft.NETCore.App\10.0.0\corerun.exe	Int32	64.50 ns	0.456 ns	0.405 ns	0.63
GetConverter	Job-JKKEPZ	\fcall\shared\Microsoft.NETCore.App\10.0.0\corerun.exe	Int32	47.09 ns	0.197 ns	0.154 ns	0.46
GetConverter	Job-EVGRXV	\good\shared\Microsoft.NETCore.App\10.0.0\corerun.exe	Int32	69.80 ns	0.405 ns	0.379 ns	0.69

LGTM

[alexey-zakharov]

thanks for the optimization!
it is amazing that the GetConverter method is now even faster than originally 🥳

[ericstj]

thanks for the optimization! it is amazing that the GetConverter method is now even faster than originally 🥳

It's curious that the combination of these changes resulted in a net speed-up, but I do consistently measure that. Benchmark code is:

        [Benchmark]
        [Arguments(typeof(int))] // primitive
        // [Arguments(typeof(int?))] // nullable primitive
        // [Arguments(typeof(Enum))] // enum
        // [Arguments(typeof(Guid))] // built-in value type
        // [Arguments(typeof(string))] // built-in reference type
        public TypeConverter GetConverter(Type typeToConvert) => TypeDescriptor.GetConverter(typeToConvert);

All changes were based off a starting commit of b6127f9. That was the "bad" data point - using a local build. "good" was after reverting 6a8720f, same process as bad. Cache and fcall were based on the same starting commit, applying mine and @jkotas's changes respectively. I would have expected us to get close to good but not exceed it, perhaps other collection changes (such as to WeakHashTable) improved this - or maybe some quirk of codegen?

[alexey-zakharov]

perhaps other collection changes (such as to WeakHashTable) improved this

WeakHashTable change could be related as the old approach was to wrap a key into a WeakReference with custom equality operator that does liveness check. I would expect ConditionalWeakTable being faster in this case.
If it is a codegen surprise, then I think it should be visible in the test stability later.