Code Generation fails for non-BCL collections

[lukasreuter]

I tried out to use the SourceGenerator but it fails at two points when used with types that are not List<> or an array:

• the first issue is in CodeGenUtils.cs @406:77 , simple fix, just the wrong symbol used in the lambda
• the other issue I could also track down but I am not sure how to fix it correctly: If you use a user implemented collection type (I was testing NativeArray for a Unity project, would be an awesome addtion for bursted code :)) then in LinqGenExpression.cs @49:13 the semantic model will always return an empty SymbolInfo for the member access. I am not quite sure why that happens but I assume its because the syntax trees are parsed seperately and the tree that contains the NativeArray has not yet been parsed?

Sorry that I don't have a pull request but SourceGenerators are still quite new to me.

[cathei]

Thanks for testing it out and creating issue! It'd be great to be burst-compatible, I'll test it and see if there is any way to support NativeArray.

[neon-sunset]

Can confirm that the issue also reproduces on net6/7 with custom IEnumerable-implementing type. But still, thank you for making this library, I will try looking into the issue too but same as OP - not familiar with SrcGen :)

[cathei]

I have solved issues mentioned, and updated version to 0.0.3-preview. Also made a simple example with Burst compile. Though, because type covariance is not supported for value types, currently only supported struct enumerable type is NativeArray<T>.

To support custom struct enumerable, it will require little work from user's end. Currently thinking of good API for this. Also currently Burst does not work with anything requires ArrayPool - will see how we can support this!

[lukasreuter]

Sweet, thanks a lot for implementing this fix :). I'll give it a spin this weekend and give you any feedback. One thing I noticed was that NativeArray is actually part of the engine and not the collections package, so I got an error on a fresh project even though NativeArray is available.

[cathei]

Oh! Never used it without the package and since it's in Unity.Collections I completely thought it was part of package .. 🤦 Thanks for letting me know! For now error should not happen if collection package is installed.

[lukasreuter]

I finally came around to testing it some and I have to say the results are impressive :). In some it manages to be faster than the naive loop implementation but others are not that most likely cannot be correctly vectorized (looking at the Max/Min impl seems like the iterator cannot be completely optimized out).

I added the other two main Collection types that are used in Burst - NativeList and FixedList - in the pull request #5, that should cover 90% of usage of it.

Regarding using ArrayPool: is that even necessary when using it with a native collection? Allocating memory in Burst/Jobs using Allocator.Temp is pretty fast (and deallocation is even faster as it uses an arena allocator) so would it be possible to use that in those cases where you would normally use a Pool?

Profiling results (Median is the most interesting):	Index	Test Name	Version	SampleGroup Name	Unit	IncreaseIsBetter	Min	Max	Median	Average	StandardDeviation	Sum
0	PerfComparison.MaxForLoop	1	µs	Microsecond	False	9.9999999999909	1005.50000000001	10.2000000000046	10.8531699999999	14.3136612350266	108531.699999999
1	PerfComparison.MaxLinqGen	1	µs	Microsecond	False	9.9999999999909	1005.50000000001	72.2000000000662	41.9974200000023	32.7600633904086	839948.400000046
3	PerfComparison.SquareAndSumForLoop	1	µs	Microsecond	False	9.9999999999909	1005.50000000001	17.799999999994	29.1328825000062	26.5497616748499	1165315.30000025
4	PerfComparison.SquareAndSumLinqGen	1	µs	Microsecond	False	9.9999999999909	1005.50000000001	14.2000000000166	26.1702420000094	24.4839369681725	1308512.10000047

[cathei]

Thanks for the update and contribution! Yes, proper vectorization would be nice to have, but it is not implemented properly for all operations. If you can share the source of benchmark I'll look more into it!

To answer the question about ArrayPool, current implementation does not use ArrayPool for unmanaged types. Despite the name PooledListNative... Instead it does use UnsafeUtility.Malloc with Allocator.Temp for Unity version, and Marshal.AllocHGlobal for NuGet version. (This was mandatory for Burst support as it does not support regular C# array) So I think what you want is already there 😄

[lukasreuter]

Thanks for the update and contribution! Yes, proper vectorization would be nice to have, but it is not implemented properly for all operations. If you can share the source of benchmark I'll look more into it!

To answer the question about ArrayPool, current implementation does not use ArrayPool for unmanaged types. Despite the name PooledListNative... Instead it does use UnsafeUtility.Malloc with Allocator.Temp for Unity version, and Marshal.AllocHGlobal for NuGet version. (This was mandatory for Burst support as it does not support regular C# array) So I think what you want is already there 😄

Well that is great to hear, seems you already solved it 😁👍

You can find the benchmark code here: https://gist.github.com/lukasreuter/12fbc01714924b5a75fd1a987eb85f5b

In any case I think the original is issue fixed and you can feel free to close it.