Make typeof(T).IsValueType more efficient

[jamesqo]

It would be nice to have a cost-free API to check whether a generic type is a class. You can check default(T) != null and be 100% confident something is a struct if that condition is true, but if default(T) == null then it could be a class or a nullable. The only way to tell right now is via reflection, using something like typeof(T).IsGenericType && typeof(T).GetGenericTypeDefinition() == typeof(Nullable<>).

I did want to use such an API several times in the past, although I can't recall where. One scenario I can think of though is if you want to treat T as an object if T is a class, but avoid boxing for structs.

Does this sound like a good idea?

Moved from https://github.com/dotnet/corefx/issues/16209

/cc @benaadams @jkotas

[jkotas]

The only way to tell right now is via reflection

Why it needs to be this complex? Can't it be just typeof(T).IsValueType ?

[benaadams]

Counter part of IsReferenceOrContainsReferences<T>() of IsReference<T>()?

[jamesqo]

typeof(T).IsValueType

@jkotas Well yes, you could use something like default(T) != null || typeof(T).IsValueType to make the check for regular structs fast and still handle nullables (or default(T) == null && typeof(T).IsValueType to detect them), but most code I've seen tends to use what I've written for explicitly detecting nullables. Either way though reflection still needs to be used.

[jamesqo]

@jkotas As an aside, it looks like the implementation could be changed in Comparer/EqualityComparer to make detecting nullables more efficient... see EqualityComparer and Comparer

[jkotas]

So the real problem is that typeof(T).IsValueType is slow. We can either tech the JIT to recognize it as intrinsic; or introduce a new API and tech the JIT to recognize the new API as intrinsic. The first option sounds better to me. Is there a good reason why we would want to introduce a new API for this instead of just make the existing one more efficient?

it looks like the implementation could be changed in Comparer/EqualityComparer

Micro-optimizations in these methods are not interesting. These methods are executed just once; and micro-optimizations won't change how expensive they are.

[jamesqo]

We can either tech the JIT to recognize it as intrinsic; or introduce a new API and tech the JIT to recognize the new API as intrinsic. The first option sounds better to me. Is there a good reason why we would want to introduce a new API for this instead of just make the existing one more efficient?

How complicated would it be to teach the JIT to recognize existing code as intrinsic? I remember seeing an issue raised by @omariom in coreclr that was dated 2015 about recognizing these kinds of patterns; can't find it now, but I came over it a year later and it was still open.

[jkotas]

How complicated would it be to teach the JIT to recognize existing code as intrinsic?

Add new CorInfoIntrinsics enum in inc\corinfo.h; recognize on the VM side; expand it impIntrinsic in the JIT. For example, here is a change that added new intrinsics for Span<T> dotnet/coreclr@0bedde3 (the commit also contains some other unrelated changes).

[omariom]

@jamesqo

I remember seeing an issue raised by @omariom in coreclr that was dated 2015 about recognizing these kinds of patterns;

Make JIT time constants from calls to Type methods and properties

[joperezr]

Seems like what this issue is about is to make the existing check more efficient, so changing title to match that plus adding the label of enhancement.