Cache the value for IsCollectible

[ericstj]

Fixes #119499

@jkotas let me know what you think. It turns out IsCollectible wasn't fast enough to put on this hot path. I'm assuming it's safe to cache it for a type and I found other fields were already cached. I made it cached lazily so that we don't impact every user of reflection that hits this cache.

CC @alexey-zakharov

[Copilot]

Pull Request Overview

This PR improves performance by caching the IsCollectible property value for RuntimeType instances. The change addresses a performance issue where the IsCollectible property was being called frequently on a hot path but wasn't fast enough due to its underlying P/Invoke call.

Key changes:

• Adds lazy caching of the IsCollectible value in the RuntimeTypeCache class
• Replaces the direct P/Invoke implementation with a cached property accessor

[Copilot]

The lazy initialization of m_isCollectible is not thread-safe. Multiple threads could simultaneously check HasValue, find it false, and then race to set the value. Consider using Volatile.Read/Volatile.Write or a lock to ensure thread safety, or accept that the P/Invoke might be called multiple times initially if that's acceptable for this use case.

[jkotas]

This is valid feedback

[jkotas]

The usual way to deal with this problem is a tri-state enum like this:

runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/TextInfo.cs

Lines 22 to 27 in 8e80e90

	private enum Tristate : byte
	{
	NotInitialized = 0,
	False = 1,
	True = 2
	}

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

[jkotas]

@jkotas let me know what you think.

I would rather fix this by making the FCall faster.

[jkotas]

I would rather fix this by making the FCall faster.

#119743

[jkotas]

@EgorBot -intel

using BenchmarkDotNet.Attributes;

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

[jkotas]

#119743 makes the implementation even faster than the reflection cache while avoiding the extra memory footprint of the reflection cache.

[ericstj]

Ok. I will close this for now. I'll benchmark your PR in the impacted TypeConverter benchmark this was fixing.