You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
@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.
@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
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.
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.
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).
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.
Activity
jkotas commentedon Feb 16, 2017
Why it needs to be this complex? Can't it be just
typeof(T).IsValueType?benaadams commentedon Feb 16, 2017
Counter part of
IsReferenceOrContainsReferences<T>()ofIsReference<T>()?jamesqo commentedon Feb 16, 2017
@jkotas Well yes, you could use something like
default(T) != null || typeof(T).IsValueTypeto make the check for regular structs fast and still handle nullables (ordefault(T) == null && typeof(T).IsValueTypeto 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 commentedon Feb 16, 2017
@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 commentedon Feb 16, 2017
So the real problem is that
typeof(T).IsValueTypeis 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?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 commentedon Feb 16, 2017
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 commentedon Feb 16, 2017
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 commentedon Feb 16, 2017
@jamesqo
Make JIT time constants from calls to Type methods and properties
joperezr commentedon Mar 16, 2017
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.
[-]Add an API to check whether a generic type is a class[/-][+]Make typeof(T).IsValueType more efficient[/+]