Collections.Pooled icon indicating copy to clipboard operation
Collections.Pooled copied to clipboard

Support for .NET Standard 2.1

Open jtmueller opened this issue 5 years ago • 15 comments

.NET Standard 2.1 is in Preview (as part of .NET Core 3.0) as of this writing. The current version...

  • Is implemented by .NET Core 3.0 but not .NET Core 2.1 or 2.2.
  • Adds Span, Memory, ArrayPool, and support for the C# 8 Index and Range syntax
  • Fully supports everything Collections.Pooled needs.

The downside is this:

  • Currently we target .NET Standard 2.0 (with NuGet packages for Span and ArrayPool) and .NET Core 2.1 (with a higher-performance built-in Span)
  • It would be nice to target only .NET Standard 2.0 and 2.1, without targeting any specific frameworks. However, this would mean that .NET Core 2.1/2.2 clients (which don't support .NET Standard 2.1) would get the Standard 2.0 version of the library with the slower implementation of Span, even though there's a faster one built in. Maybe type redirection would cause it to use fast-Span regardless, but I'm not sure.
  • However, keeping the .NET Core 2.1 target while adding .NET Standard 2.1 is also a problem. A .NET Core 3.0 client (at least as of the current preview) will default to loading the .NET Core 2.1 version of the library when presented with a choice between Core 2.1 and Standard 2.1. I guess that specific frameworks trump standards, even when the supported standard is newer. This means that .NET Core 3.0 clients would not get the version of the library that supports Indexes and Ranges.
  • To fix that last issue, I would have to target four frameworks: Standard 2.0 and 2.1, Core 2.1 and 3.0. This list would likely keep growing over time. I don't want to do this, because it complicates conditional compilation, and I'd rather target a small number of standards than many different versions of .NET Core.

My current plan is this, when the final release of .NET Core 3 comes out:

  • Increment the major version number, as this is a breaking change for .NET Core 2.1/2.2 users.
  • Target only .NET Standard 2.0 and 2.1.

By incrementing the major version, .NET Core 2.x users will not auto-upgrade to the new release, and can keep using Collections.Pooled 1.x. Users of .NET Core 3 will get the Standard 2.1 version, and all other users will get Standard 2.0 (until another framework adds support for Standard 2.1 of course).

This code is currently fully implemented in the core-3 branch if you want to experiment with it.

@dzmitry-lahoda and @Wixred I'm tagging you because you've shown an interest. Do you have any objection to this plan, or any alternate suggestions?

jtmueller avatar Mar 28 '19 01:03 jtmueller

I agree with the plan proposed to target only .NET Standard 2.0 and 2.1.

Wixred avatar Mar 28 '19 03:03 Wixred

We are on Unity which is standard 2.0 and will be 2.1. On server, because of some legacy, we still .net full, which is standard 2.0 and will be 2.1. So for us standard 2.0 best, until 2.1 released for unity and full. But we I fine to have own modified copy, so better for clean library code with less #if.

dzmitry-lahoda avatar Mar 28 '19 05:03 dzmitry-lahoda

Sounds good, thanks guys. Once the final .NET Core 3.0 comes out I'll re-run benchmarks and publish the Index/Range supporting version targeting Standard 2.0 and 2.1 with a major version bump on the Collections.Pooled package.

@dzmitry-lahoda It is my understanding that .NET Framework will never support .NET Standard 2.1 or later, and is essentially in maintenance-only mode at this point. If you come across information to the contrary, I would love to see it.

jtmueller avatar Mar 29 '19 03:03 jtmueller

https://blogs.msdn.microsoft.com/dotnet/2018/10/04/update-on-net-core-3-0-and-net-framework-4-8/

Given many of the API additions in .NET Standard 2.1 require runtime changes in order to be meaningful, .NET Framework 4.8 will remain on .NET Standard 2.0 rather than implement .NET Standard 2.1. .NET Core 3.0 as well as upcoming versions of Xamarin, Mono, and Unity will be updated to implement .NET Standard 2.1.

You named nothing which requires runtime changes to build and run(may be with previous not ideal performance). I guess, after silent msbuild about bad dependency, I will be able to run as usual. If not than will #if that code until we upgrade.

dzmitry-lahoda avatar Mar 29 '19 06:03 dzmitry-lahoda

If Span is as fast as Array in .NET Core 3.0 (not sure of other runtimes as of now), may be it could be considered to use MemoryPool, instead of ArrayPool. This will allow allocated via native memory without any runtime hacks (as of now I can allocate native, and hack it to pretend it to be array, but that not portable).

dzmitry-lahoda avatar Apr 10 '19 11:04 dzmitry-lahoda

If you review the source code for MemoryPool, it's just a wrapper around ArrayPool that implements IDisposable to allow you to return arrays to the pool with the using pattern. I experimented with doing everything with Span in PooledList at the beginning (.NET Core 2.1), but that was actually slower than working directly with arrays, so I reverted the code back to ArrayPool. Are there optimizations in Core 3.0 that were not present in 2.1?

Or am I misunderstanding you, and you're saying you'd like to provide a custom MemoryPool to PooledList that is based on something other than arrays, rather than using the default MemoryPool? That's an interesting idea, but I'm kind of reluctant to hurt performance in the typical use case, especially for people stuck on .NET Framework at their jobs (like me) for the sake of something relatively exotic like that.

Span itself has runtime optimizations in .NET Core 2.1-3.0 that make the Span built in to those frameworks faster than the one in the System.Memory NuGet package for .NET Standard 2.0. That's the reason I brought this topic up, because after switching to supporting only .NET Standard, that means that .NET Core 2.1 and 2.2 should stick with Collections.Pooled 1.x if they want access to the fast version of Span. Or maybe internal type redirects would cause Core 2.1 and 2.2 to redirect to the built-in version of Span even when using the Standard 2.0 version of Collections.Pooled, I'm not really sure.

If there's a compelling case for a custom MemoryPool that's not based on arrays, one option might be to clone PooledList into MemoryPooledList and convert that code over to work with MemoryPool instead of ArrayPool. I'd be really curious to learn more about such a MemoryPool.

jtmueller avatar Apr 13 '19 21:04 jtmueller

I may implement custom memory pool integrated with Unity event loop https://github.com/dzmitry-lahoda/memory-pool-unity-unsafe . So exotic case could be games. But of course not for the price of common case.

MemoryPool seems to be provider of Memory struct with Span. So _items access replaced with _memory.Span access. The case, .NET Core 3.0 runs project compiled for .NET Standard 2.1. If the case will have 1 to 1 performance for both access patterns. What technical obstacles will prevent using memory/span? Keeping aside that people should stick with old version of Collection.Pooled for performance on old runtime(which actually they do regardinh other stuff in corefx when sticking to old run time).

May be we will sometime in future clone source, by may be not sooner than half year. Also it is interesting how to have pooled and not pooled without copying whole source https://github.com/dotnet/corefxlab/issues/2654 . And if pooled then pooled on GC and native memory. So may be will check if .NET Core 3.0 optimizes these things out.

dzmitry-lahoda avatar Apr 14 '19 05:04 dzmitry-lahoda

Weird...

SimpleGenericFastAsManual - array storage in same project as test bench Generic7Bit - memory-span storage backed by array in net standard 2.0 project, same fast when used array ( from https://github.com/dzmitry-lahoda/NetStack/commit/38d8408439456ae8f1ad0a9b43adf12bab0cb183)

On Core 3.0 (<TargetFramework>netcoreapp3.0</TargetFramework> in bench):

BenchmarkDotNet=v0.11.5, OS=Windows 10.0.17134.706 (1803/April2018Update/Redstone4)
Intel Core i7-7700HQ CPU 2.80GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.0.100-preview3-010431
  [Host] : .NET Core 3.0.0-preview3-27503-5 (CoreCLR 4.6.27422.72, CoreFX 4.7.19.12807), 64bit RyuJIT
  Core   : .NET Core 3.0.0-preview3-27503-5 (CoreCLR 4.6.27422.72, CoreFX 4.7.19.12807), 64bit RyuJIT

Job=Core  Runtime=Core  InvocationCount=1  
UnrollFactor=1  

|                    Method |     N |     Mean |     Error |    StdDev |   Median |
|-------------------------- |------ |---------:|----------:|----------:|---------:|
| SimpleGenericFastAsManual | 10000 | 6.908 ms | 1.5528 ms | 4.5783 ms | 4.090 ms |
|               Generic7Bit | 10000 | 5.621 ms | 0.3028 ms | 0.8640 ms | 5.811 ms |
|                NoEncoding | 10000 | 6.737 ms | 0.3654 ms | 1.0716 ms | 6.939 ms |

On Core 2.2 (<TargetFramework>netcoreapp2.2</TargetFramework> in bench):

BenchmarkDotNet=v0.11.5, OS=Windows 10.0.17134.706 (1803/April2018Update/Redstone4)
Intel Core i7-7700HQ CPU 2.80GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.0.100-preview3-010431
  [Host] : .NET Core 2.2.3 (CoreCLR 4.6.27414.05, CoreFX 4.6.27414.05), 64bit RyuJIT
  Core   : .NET Core 2.2.3 (CoreCLR 4.6.27414.05, CoreFX 4.6.27414.05), 64bit RyuJIT

Job=Core  Runtime=Core  InvocationCount=1  
UnrollFactor=1  

|                    Method |     N |      Mean |     Error |    StdDev |   Median |
|-------------------------- |------ |----------:|----------:|----------:|---------:|
| SimpleGenericFastAsManual | 10000 |  4.994 ms | 0.3005 ms | 0.8812 ms | 4.643 ms |
|               Generic7Bit | 10000 |  7.958 ms | 0.2723 ms | 0.7855 ms | 7.970 ms |
|                NoEncoding | 10000 | 10.032 ms | 0.3244 ms | 0.9464 ms | 9.900 ms |

Array usage became slower in .NET Core 3.0... And Span on 3.0 is faster (as expected).

dzmitry-lahoda avatar Apr 14 '19 13:04 dzmitry-lahoda

There is something https://github.com/dotnet/coreclr/issues/23620 . So changing runtime may slow array access (may be because of JIT recompilations).

BDN runs much slower on 3.0. I have increased N size and got normal results. Memory.Span is slower.

dzmitry-lahoda avatar Apr 14 '19 13:04 dzmitry-lahoda

Given one more generic parameter into list, it seems possible to have both (array and memory) solutions in one code base. So need to test on list. Idea is wrap array and memory in structure with accessors. Make ListBase<T, TStorage> where TStorage : struct, IStorage. Make PooledList <T> : ListBase<T, ArrayStorage>.

Storage is like (and any other methods needed):

   public struct data2 : ISpan
    {
        public Memory<uint> chunks;

        public uint this[int index]
        {
            [MethodImpl(MyMethodImplOptions.AggressiveInlining)]
            get => chunks.Span[index];
            [MethodImpl(MyMethodImplOptions.AggressiveInlining)]
            set => chunks.Span[index] = value;
        }

        public int Length
        {
            [MethodImpl(MyMethodImplOptions.AggressiveInlining)]
            get => chunks.Length;
        }

    }

Example https://github.com/dzmitry-lahoda/NetStack/commit/8fd9cda8eedaff1e641fd8478cec7bb7ac868b33 . Works in my case on 2.2.

As of .NET Core 3 preview 2. I did quick view of coreclr issues regarding degrade of performance - there are some for loops-list-arrays-sets. So I guess only with release (or event with next release) (inlcuding BDN new release) it could be possibly resonably measure performance on 3.0.

I guess approch may allow to have PooledList, List, PooledMemoryList to inherit one codebase of ListBase :)

dzmitry-lahoda avatar Apr 14 '19 16:04 dzmitry-lahoda

I had originally converted PooledList over to work in terms of Span. I thought it would be just as fast, and with much simpler code, as the Span API is nicer. However, it proved to be slightly slower than arrays in the List benchmarks in .NET Core 2.1, and enough slower in .NET Framework 4.6.1 that I decided to change all the code back to working with arrays to avoid that overhead.

Given that I need to support .NET Framework, I'd like to keep PooledList based on raw arrays for performance on that platform. I'm not opposed to making a new class based on PooledList with MemoryPool/Memory/Span, even if that does mean duplicated code. If side-by-side benchmarks prove me wrong about performance overhead on .NET Framework, I'll be happy to switch over to MemoryPool only.

jtmueller avatar Apr 15 '19 08:04 jtmueller

So you want .NET Framework 4.6.1 with full performance? Or will be OK with 4.7.2 or 4.8? Or if .NET Core 2+ is fast, but full framework of 4.7.2 and less is lower?

I think I can have single codebase, retaining array performance, but interested only in .NET Core 2+ and .NET Framework 4.8.

I mean, that 4.6.1 may lack optimization I used, and will be slower.

dzmitry-lahoda avatar Apr 15 '19 09:04 dzmitry-lahoda

I mention 4.6.1 because that is the lowest version of .NET Framework that supports .NET Standard 2.0. If Collections.Pooled supports .NET Standard 2.0, it needs to work as well as possible on .NET Framework 4.6.1.

The version of Span built-in to .NET Core 2.1 and above has some optimizations enabled at the runtime level that the version of Span in the System.Memory NuGet package cannot have, because .NET Standard 2.0 doesn't support those particular optimizations. In particular, .NET Framework is not likely to ever have these optimizations, including 4.8. The difference is often not huge, but it is there.

I'm happy to start work on a version of PooledList that's based on MemoryPool instead of ArrayPool, to allow for custom MemoryPool implementations. I'm just not prepared to replace the array-based version of PooledList with the MemoryPool version until I can do some side-by-side benchmarks on both .NET Core and .NET Framework and see whether performance is comparable.

jtmueller avatar Apr 19 '19 05:04 jtmueller

I think will replace array with memory locally if need that. And will wait to measure until release of .NET Full 4.8 and .NET Core 3.0 to see what is possible.

dzmitry-lahoda avatar Apr 26 '19 08:04 dzmitry-lahoda

@jtmueller is this project still maintained? I would love to see improvements targeting .NET 5 / .NET Standard 2.1+. Since Unity and other runtimes are being deprecated/replaced with .NET 6, I think maintaining backward compatibility is moot. Personally I would probably rewrite this against whatever is in the .NET 5 source code and release it as a new major version that is specifically not targeting the Span/Memory nuget or old .NET Framework.

I would love to use this for my project since it is badly needed. Thanks!

kamronbatman avatar Jan 17 '21 17:01 kamronbatman