Add RuntimeHelpers.IsReferenceOrContainsReferences<T>()

[jkotas]

namespace System.Runtime.CompilerServices
{
    public static class RuntimeHelpers
    {
        public static bool IsReferenceOrContainsReferences<T>();
    }
}

This method will return true if T is reference type; or if T is value type that contains reference type fields. false otherwise.

This API is intended to be used primarily for optimizations:

• Generic List calls Array.Clear on its underlying array in the implementation of Clear, RemoveAll, RemoveRange methods today. It has to do so because the generic argument can contain references which must be freed for GC. It can be skipped using this method when not required.

• Algorithms for memory manipulations can have optimized paths for memory blocks without GC references. This method can be used to determine whether the optimized path is safe to use. Span<T> is example of such memory manipulation algorithms. It going to use this API internally in its implementation.

This API addition was tracked by CoreCLR issue. https://github.com/dotnet/coreclr/issues/1135 has a lot of discussion about the right name for this API. The alternative names considered include ContainsReferences, IsGCPointerOrContainsGCPointers.

[jkotas]

cc @omariom @nietras

[benaadams]

In Jit terms this and its associated branch can always be elided? Shared code is always a reference; and value type code is always unique per value type?

[jkotas]

In Jit terms this and its associated branch can always be elided?

Correct. This method will be intrinsic that the JIT can optimize out.

[nietras]

@jkotas should this employ the caching behaviour internally (e.g. via PerTypeValues<T> or TypeCache<T>) here (for the portable version)? Or are "clients" responsible for this?

I would think it best to have the cache as early as possible so there is only one cache.

[jkotas]

There is no point in having the cache if it is JIT intrinsic. And even if it is not JIT intrinsic, it would be just fetching a few bits from MethodTable* - no point in caching it either.

Note that there won't be a public portable version of this method. The implementation using reflection will be internal to Span<T>.

[masonwheeler]

@jkotas

Note that there won't be a public portable version of this method. The implementation using reflection will be internal to Span.

Huh? In your example, it's marked as public.

[jkotas]

Huh? In your example, it's marked as public.

Right, this issue is about adding a new public .NET Core API implemented efficiently by the runtime, that may come to .NET Framework in future version. The slower portable Span implementation cannot use this public API because of it needs to run on existing runtimes - that's why it needs internal implementation of the API using reflection.

[masonwheeler]

Ah, I see. That makes sense.

[terrajobst]

The API makes perfect sense.

@jkotas: should we add a non-generic version that takes Type? This might be useful for implementing serializers. Having to call a generic API would be quite painful.

The only thing is that we believe we should name this concept, e.g. blittable. There are likely other areas where we'd use this nomenclature (e.g. casting from Span<char> to Span<byte>). It would be very unfortunate if the naming doesn't align.

@MadsTorgersen @jaredpar, have you guys thought about naming this concept?

[benaadams]

blittable is a bit different though? e.g. Decimal and DateTime are not blittable; but they also are not reference types

[jkotas]

Right, blittable is existing concept, different from this one, specific to interop - msdn definition: https://msdn.microsoft.com/en-us/library/75dwhxf7(v=vs.110).aspx

[jkotas]

The C# spec calls similar concept unmanaged-type, but that concept has a known ugly problems (spooky action at a distance, etc.). I do not think it makes sense to couple this property to it either.

[jaredpar]

Explicit blittable support is one item we're confsidering for the next version of C#. It would solve the spooky action by making usages explicit and verified.

I don't think it would work for this feature though. The C# feature is a compile time artifact while the methods @jkotas listed need to make decisions at runtim.

[nietras]

@jkotas ha so much I am still not partial too, so have some questions.

no point in having the cache if it is JIT intrinsic. And even if it is not JIT intrinsic, it would be just fetching a few bits from MethodTable* - no point in caching it either.

I understand that for JIT intrinsic it does not matter, but when it is not, will it be implemented in coreclr? So there won't be a "portable" version of this, for others e.g. me ;) to use?

Note that there won't be a public portable version of this method. The implementation using reflection will be internal to Span.

So... this can't be used in Span<T>, what about List<T>, Dictionary<TKey, TValue> etc.?

blittable, reference free value type, unmanaged, primitive etc.

As I noted in another issue, I do believe there needs to be better qualification/clear naming of these "concepts" as these are often confused with eachother. F# uses unmanaged for the "pure" value type concept. Others suggest primitive but that is also wrong. And so forth. A document explaining these and vocabulary would be nice. And of course having this concept "coined" with a new term would be great too.

[jaredpar]

As I noted in another issue, I do believe there needs to be better qualification/clear naming of these "concepts" as these are often confused with eachother.

Agree. Part of the reason there is no central doc is because a lot of these issues are in the very early stages. We haven't committed to doing them and hence haven't done a lot of work trying to get coherent docs in order.

F# uses unmanaged for the "pure" value type concept. Others suggest primitive but that is also wrong.

The term unmanaged is certainly more ubiquitous given it is also how the C# spec refers to such types: unmanaged types specifically. It's a strong possibility for what we'd call the C# feature if implemented.

Terms like blittable / primitive are just used today because there are existing implementations / presentations that we are looking at. It's how Midori implemented this feature and hence there is a deal of existing vocabulary that we are using.

[benaadams]

unmanaged has connotations of data sourced external to the clr; rather than data that doesn't contain GC references. The type if it was a field in a class would also be managed in these terms. Might just be me though...

[jkotas]

I agree with @benaadams

The early concepts have nothing to do with this API. It is pretty unlikely that any of them will match what this API does.

This API just checks whether the valuetype contains anything tracked by the GC, nothing else. It is meant to allow libraries to do certain operation more efficiently if it does not - the write up about intended use at the top has more details.

[nietras]

unmanaged has connotations of data sourced external to the clr

I completely agree, wasn't suggesting this was better than other. Just that we need a new unambiguous term, for future. I think naming of IsReferenceOrContainsReferences is fine for now, since it will probably take some time before a better term can be agreed upon.

[mattwarren]

This API just checks whether the valuetype contains anything tracked by the GC, nothing else.

Couldn't that be the name then, 'IsTrackedByGC()'? (or something along those lines) On 29 Nov 2016, at 19:48, "Jan Kotas" <notifications@github.com> wrote:

[masonwheeler]

I've heard it said there are only two hard problems in computer science: Cache invalidation, naming things well, and off-by-one errors...

[terrajobst]

[@nietras] I think naming of IsReferenceOrContainsReferences is fine for now, since it will probably take some time before a better term can be agreed upon.

With APIs there is almost never a "for now". Once in, we can rarely change the APIs (unless it's a preview). Just going with a random name because we can't think of one "right now" isn't a good enough reason to accept breaking changes later, though.

@jkotas @jaredpar: To be clear, I wasn't suggesting to use an existing concept for the name. My desire is that we should get our terminology straight. As both of you pointed out, the space is confusing precisely because we never bothered to make these things first class concepts with a better name.

[nietras]

almost never a "for now"

@terrajobst of course and I agree that the best case would be to get these concepts named for once.

Perhaps I also misunderstood @jkotas when he said this would not be "public", since it was probably more related to not being "public" in a portable sense. Still juggling with these different terms too... what's .NET Core specific, what's .NET Standard i.e. portable API?, what's part of coreclr, whats part of corefx, what's part of both etc. And when are we getting all the cool stuff in .NET Framework? Lots to learn, which is great. Just wish there was a better introduction to this ;)

[jkotas]

when it is not, will it be implemented in coreclr? So there won't be a "portable" version of this, for others e.g. me ;) to use?

@nietras Correct. It is consequence of adding it to existing type. It cannot be added to existing runtimes that have the type already.

Couldn't that be the name then, 'IsTrackedByGC()'?

@mattwarren I had similar observation earlier - my suggestion was ContainsGCPointers<T>(). Check the discussion about it at https://github.com/dotnet/coreclr/issues/1135#issuecomment-262616750.

[nietras]

It is consequence of adding it to existing type

@jkotas would it not make sense to add it somewhere else then? Not that I couldn't reimplement it in the places I would like to use it, but it would be good to have in a portable way, in the future at least...

[jkotas]

These methods should come to other runtimes in future, and become part of the portable set that you can depend on.

There is a challenge with adding one-off methods, and making them available on existing runtimes: We would need to create a new type and likely even a new assembly for it. This approach is ok for large chunks like ValueType or Span<T>, but it does not scale for one-off methods. We would end up with a lot of very similar types that have just a few methods on them over time. Copy&paste the implementation to make it work on older runtimes is probably the best of the bad choices. Or having it in a small 3rd party nuget package that is not part of the platform.

[karelz]

API triage: Ideally we would like to see this method as internal. Span might need it public, we should just wait for its design to settle a bit more, then we can review it again.

[jkotas]

Span might need it public

Span does not need it public. It is needed as public for everything else that can take advantage of it, like the optimization in generic collections mentioned at the top of this issue.

This API exits as internal in both CoreCLR and CoreFX already (under different names):

• CoreCLR: System.Runtime.CompilerServices.JitHelpers.ContainsReferences<T>(): https://github.com/dotnet/coreclr/blob/f42984160f5f9407f70ebc2e86edfcf226fccd81/src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs#L239

• CoreFX: System.SpanHelpers.IsReferenceFree<T>: https://github.com/dotnet/corefx/blob/ccc44b7b2e9145251e2ca98bbe0936938ed94379/src/System.Memory/src/System/SpanHelpers.cs#L50

• There are blog posts and stackoverflow threads about the best workaround to get this property.

[karelz]

@jkotas you should come next week to API review meeting to close on this.

[terrajobst]

API looks fine. We're not concerned about the language design because they are likely going down a different path. We also considered adding an overload taking Type but concluded we don't need it yet.

[jamesqo]

Should this issue be closed?

[jkotas]

It was not exposed in the contracts yet.