alanmcgovern / Mono.Nat

UPNP and NAT-PMP port forwarding for .NET
https://github.com/mono/Mono.Nat
MIT License
163 stars 156 forks source link

Reduce number of allocations #19

Closed Bond-009 closed 4 years ago

Bond-009 commented 4 years ago

The Encode function could be improved a lot if the netstandard version gets increased to 2.1

alanmcgovern commented 4 years ago

If you'd like to use multitargeting and, #if NET_2_1 conditional compilation, we could ship an improved version for NS2.1 and retain support for NS2.0!

alanmcgovern commented 4 years ago

Otherwise the patch looks good. I'll have to review properly in a week or so

Bond-009 commented 4 years ago

Done

alanmcgovern commented 4 years ago

Looks good to me, thanks for the patches.

The only qualm I have is adding a dependency on the System.Memory support package for NS 2.0. At this point in time my guess is that most application targeting NS 2.0 are going to be using the desktop framework and so will never have a built-in System.Buffers.ArrayPool. As such there's a low likelihood that NS2.0 applications will gain any benefit from ArrayPool specifically.

I've tweaked the code slightly to use ArrayPool for everything that is not NS 2.0, and for NS 2.0 it caches a single buffer. This way we do not have to add a dependency on System.Memory for NS2.0.

Thanks!