Resolved Non allocating (pooled) get components iterator

Can we get a non allocating(pooled) iterator for GetComponents?
I am asking for something like this

Code (CSharp):

1.  
2.             foreach (var item in this.ComponentsInChildrenQuery<Transform>(true))
3.             {
4.                
5.             }
6.  

We currently have a inhouse solution for this that uses GetComponentsInternal and passes a pooled list of
List<object> (not List<T> since we do the casting to T in the iterator also this allows us to have only one pool) but that's hacky since it relies on internal code.

 

Yes that is totally doable but what i did not like is that we require a pool for each generic type <T>. Having access to GetComponentsInternal allows us to share the pool and still get some type safety.

Code (CSharp):

1. [StructLayout(LayoutKind.Auto)]
2. public struct ComponentsEnumerator<T> : IDisposable where T : class
3. {
4.     public static readonly Type Type = typeof(T);
5.     private List<object> _internalList;
6.     private int _index;
7.  
8.     public ComponentsEnumerator(GameObject gameObject, bool recursive, bool includeInactive, bool reverse)
9.     {
10.         if (gameObject == null)
11.             throw new ArgumentNullException(nameof(gameObject));
12.  
13.         _internalList = ListPool.Rent();
14.         UnityGameObjectInternals.GetComponentsInternal(gameObject, Type, false, recursive, includeInactive, reverse,
15.             _internalList);
16.         _index = -1;
17.     }
18.  
19.     public T Current
20.     {
21.         [MethodImpl(MethodImplOptions.AggressiveInlining)]
22.         get => (T)(_internalList[_index]);
23.     }
24.  
25.     [MethodImpl(MethodImplOptions.AggressiveInlining)]
26.     public bool MoveNext()
27.     {
28.         _index++;
29.         return _internalList.Count > _index;
30.     }
31.  
32.     public void Dispose()
33.     {
34.         if (_internalList == null)
35.             return;
36.         ListPool.Return(_internalList);
37.         _internalList = null;
38.     }
39. }

 

So you just gave me a idea i just realised we can do without access to GetComponentsInternal by using Unsafe.As to cast the List<object> to a List<T> and that would not require GetComponentsInternal. So i guess this is possible without internals. Thanks.

 

This is not safe and can confuse the JIT, leading to subtle aliasing issues. I'm not sure how many of these will manifest under Mono or IL2CPP, but there are very real bugs associated with this under .NET Core, so I'd keep this in mind if you plan to keep that code around when Unity moves to it.

In the absence of new APIs on Unity's side, you will indeed need to pool a unique List for each T if you want to be safe, which I would strongly suggest doing.

It's also not worth storing the result of

typeof(T)

in a variable or field, this will actually make performance/codegen worse because the JIT will not be able to optimize it to a direct access because it's being accessed indirectly.

An alternative approach would be to pool a

List<Component>

instead, which will allow you to use the normal public

GetComponents

method, and your

T Current

property on your enumerator can be implemented as

return Unsafe.As<T>(_internalList[_index]);

to avoid the cost of a "safe" cast. This usage of

Unsafe.As<T>

is fine because the value is guaranteed to be of type T (in fact, it's the only safe way to use that particular overload).

 

I don't see how this would break under net core, but ill make sure to do some tests, also i am storing typeof(T) with il2cpp in mind (all our games run under il2cpp) and based on this article https://www.jacksondunstan.com/articles/4689 (this might have changed) calling typeof() or GetType is not really free so i figured having a static field should be faster. (i know because of that static field there will be some code to handle static field initialization but should still be better than typeof)
About using a list of List<Component> i think you are right this would be in fact the best way to do it thanks for the idea.

 

It's rather complicated, you can see more details here. An example where

Unsafe.As<T>

can introduce aliasing issues is shown below (

MemoryMarshal.GetArrayDataReference

returns a reference to the first element of the array):

Code (CSharp):

1. Console.WriteLine(Problem(new byte[] { 1 }));
2.  
3. static int Problem(byte[] a)
4. {
5.     if (MemoryMarshal.GetArrayDataReference(a) == 1)
6.     {
7.         a[0] = 2;
8.         if (MemoryMarshal.GetArrayDataReference(a) == 1)
9.         {
10.             return -1;
11.         }
12.     }
13.  
14.     return 0;
15. }

MemoryMarshal.GetArrayDataReference

used to be implemented using an

Unsafe.As

cast to an incompatible reference type (

RawArrayData

). This introduced an aliasing bug: depending on optimization and various other factors, the above code can print -1 instead of 0.

It was recently changed to an intrinsic instead of using

Unsafe.As<T>

to avoid these problems.

 

Yes it does seem a bit complicated, so what you were saying is that when unity moved to net core they would be required to use MemoryMarshal.GetArrayDataReference under the hood of GetComponentsInternal and because i passed a List<object> instead of List<T> it could break the jit?

 

MemoryMarshal.GetArrayDataReference

is moreso just an example of

Unsafe.As<T>

causing problems. The problem is specifically that you're aliasing a

List<object>

as a

List<T>

, which isn't its real type and thus has the potential to elicit similar bugs under .NET Core (

GetArrayDataReference

was doing something very similar: aliasing a

T[]

as a

RawArrayData

). In other words, it's essentially undefined behavior.

 

Ok now i get what you meant, thanks for clarifying!