From 1264afb2ce60e33102a905b39a220965e8cfb4db Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Sun, 8 May 2022 05:01:32 -0400 Subject: [PATCH 1/9] feature: Add ArrayBufferPool implementation. Add an ArrayPool from the System.Buffers namespace. With it, one can exclude the System.ServiceModel.Primitives package. --- src/NetMQ/BufferPool.cs | 60 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/NetMQ/BufferPool.cs b/src/NetMQ/BufferPool.cs index dea5e6eb8..345211684 100644 --- a/src/NetMQ/BufferPool.cs +++ b/src/NetMQ/BufferPool.cs @@ -1,6 +1,7 @@ using System; using System.ServiceModel.Channels; using System.Threading; +using System.Buffers; namespace NetMQ { @@ -24,6 +25,65 @@ public interface IBufferPool : IDisposable void Return(byte[] buffer); } + public class ArrayBufferPool : IBufferPool + { + private readonly ArrayPool m_arrayPool; + + /// + /// Create a new ArrayBufferPool with the specified maximum buffer pool size + /// and a maximum size for each individual buffer in the pool. + /// + /// the maximum size to allow for the buffer pool + /// the maximum size to allow for each individual buffer in the pool + /// There was insufficient memory to create the requested buffer pool. + /// Either maxBufferPoolSize or maxBufferSize was less than zero. + public ArrayBufferPool(long maxBufferPoolSize, int maxBufferSize) + { + m_arrayPool = ArrayPool.Create(maxBufferSize, (int) maxBufferPoolSize); + } + + /// + /// Return a byte-array buffer of at least the specified size from the pool. + /// + /// the size in bytes of the requested buffer + /// a byte-array that is the requested size + /// size cannot be less than zero + public byte[] Take(int size) + { + return m_arrayPool.Rent(size); + } + + /// + /// Return the given buffer to this manager pool. + /// + /// a reference to the buffer being returned + /// the Length of buffer does not match the pool's buffer length property + /// the buffer reference cannot be null + public void Return(byte[] buffer) + { + m_arrayPool.Return(buffer); + } + + /// + /// Release the buffers currently cached in this manager. + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Release the buffers currently cached in this manager. + /// + /// true if managed resources are to be disposed + protected virtual void Dispose(bool disposing) + { + if (!disposing) + return; + // We don't have a means of clearing ArrayPool like BufferManager does. + } + } /// /// This implementation of uses WCF's /// class to manage a pool of buffers. From 444a1de094ad34cf013b63062693a2d76d3977b7 Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Sun, 8 May 2022 05:16:01 -0400 Subject: [PATCH 2/9] feature: Add a build flavor 'Minimal' In an effort to use NetMQ with Unity3D, one problem was trying to satisfy the dependencies for `System.ServiceModel.Primitives`. It required also providing `System.ServiceModel.{Http,Duplex,NetTcp}` and perhaps more before I gave up. These dependencies were only noticed while Unity3D built the project. When using NetMQ in a regular dotnet project, I've had no problems with the `System.ServiceModel.Primitives` dependencies. I added an alternative implementation of `IBufferPool` using the `ArrayPool` from `System.Buffers` but it is unused. I instrumented the build system to have a "Flavor" that by default is set to "Standard", which changes nothing from before. For my usecase, however, I set the flavor to "Minimal". $ dotnet build -p:Flavor=Minimal This excludes "System.ServiceModel.Primitives" and defines a preprocessor flag of `FLAVOR_MINIMAL` that excludes the BufferManagerBufferPool class and uses the ArrayBufferPool instead. --- src/NetMQ/BufferPool.cs | 18 ++++++++++++++++-- src/NetMQ/NetMQ.csproj | 17 +++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/NetMQ/BufferPool.cs b/src/NetMQ/BufferPool.cs index 345211684..472df15cf 100644 --- a/src/NetMQ/BufferPool.cs +++ b/src/NetMQ/BufferPool.cs @@ -1,5 +1,7 @@ using System; +#if ! FLAVOR_MINIMAL using System.ServiceModel.Channels; +#endif using System.Threading; using System.Buffers; @@ -25,6 +27,10 @@ public interface IBufferPool : IDisposable void Return(byte[] buffer); } + /// + /// This implementation of uses System.Buffer's ArrayPool + /// class to manage a pool of buffers. + /// public class ArrayBufferPool : IBufferPool { private readonly ArrayPool m_arrayPool; @@ -84,6 +90,8 @@ protected virtual void Dispose(bool disposing) // We don't have a means of clearing ArrayPool like BufferManager does. } } + +#if ! FLAVOR_MINIMAL /// /// This implementation of uses WCF's /// class to manage a pool of buffers. @@ -148,6 +156,7 @@ protected virtual void Dispose(bool disposing) m_bufferManager.Clear(); } } +#endif /// /// This simple implementation of does no buffer pooling. Instead, it uses regular @@ -194,6 +203,7 @@ protected virtual void Dispose(bool disposing) } } + /// /// Contains a singleton instance of used for allocating byte arrays /// for instances with type . @@ -204,7 +214,7 @@ protected virtual void Dispose(bool disposing) /// /// The default implementation is . /// - /// Call to replace it with a . + /// Call to replace it with a pooling implementation. /// Call to reinstate the default . /// Call to substitute a custom implementation for the allocation and /// deallocation of message buffers. @@ -223,7 +233,7 @@ public static void SetGCBufferPool() } /// - /// Set BufferPool to use the to manage the buffer-pool. + /// Set BufferPool to use a buffer pooling implementation to manage the buffer-pool. /// /// the maximum size to allow for the buffer pool /// the maximum size to allow for each individual buffer in the pool @@ -231,7 +241,11 @@ public static void SetGCBufferPool() /// Either maxBufferPoolSize or maxBufferSize was less than zero. public static void SetBufferManagerBufferPool(long maxBufferPoolSize, int maxBufferSize) { +#if FLAVOR_MINIMAL + SetCustomBufferPool(new ArrayBufferPool(maxBufferPoolSize, maxBufferSize)); +#else SetCustomBufferPool(new BufferManagerBufferPool(maxBufferPoolSize, maxBufferSize)); +#endif } /// diff --git a/src/NetMQ/NetMQ.csproj b/src/NetMQ/NetMQ.csproj index 71422794f..e85951b56 100644 --- a/src/NetMQ/NetMQ.csproj +++ b/src/NetMQ/NetMQ.csproj @@ -22,6 +22,19 @@ true snupkg true + + Standard + + + + + + $(DefineConstants);FLAVOR_MINIMAL @@ -40,13 +53,13 @@ - + - + From 8c87cbc7df7924b500a2d83de89902e4e6b47221 Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:20:02 -0400 Subject: [PATCH 3/9] test: Test both IBufferPool implementations. --- src/NetMQ.Tests/BufferPoolTests.cs | 64 ++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 src/NetMQ.Tests/BufferPoolTests.cs diff --git a/src/NetMQ.Tests/BufferPoolTests.cs b/src/NetMQ.Tests/BufferPoolTests.cs new file mode 100644 index 000000000..3a8e820f1 --- /dev/null +++ b/src/NetMQ.Tests/BufferPoolTests.cs @@ -0,0 +1,64 @@ +using System; +using Xunit; + +namespace NetMQ.Tests +{ + public abstract class BufferPoolTests + { + protected int maxBufferSize = 2; + protected long maxBufferPoolSize = 100L; + protected IBufferPool pool; + + [Theory] + [InlineData(500)] + [InlineData(50000)] + [InlineData(5000000)] + [InlineData(500000000)] + public void Rent(int size) + { + var array = pool.Take(size); + Assert.Equal(size, array.Length); + } + + // This never happens. + // [Fact] + // public void RentTooBigBuffer() + // { + // Assert.ThrowsAny(() => pool.Take(900000000)); + // } + + [Fact] + public void Return() { + var array = pool.Take(10); + pool.Return(array); + } + + [Fact] + public void ReturnUnknown() { + var array = new byte[100]; + Assert.Throws(() => pool.Return(array)); + } + } + + public class ArrayBufferPoolTests : BufferPoolTests + { + public ArrayBufferPoolTests() + { + /* The sizes were chosen to be very small. It's not clear they + * constitute any bounds on the array sizes that can be + * requested. */ + pool = new ArrayBufferPool(maxBufferPoolSize, maxBufferSize); + } + } + + public class BufferManagerBufferPoolTests : BufferPoolTests + { + public BufferManagerBufferPoolTests() + { + /* The sizes were chosen to be very small. It's not clear they + * constitute any bounds on the array sizes that can be + * requested. */ + pool = new BufferManagerBufferPool(maxBufferPoolSize, maxBufferSize); + } + } +} From f944d621207ff20674d43af2e14acc6ab6792fa4 Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:21:09 -0400 Subject: [PATCH 4/9] fix: Throw when wrong array return to pool. --- src/NetMQ/BufferPool.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/NetMQ/BufferPool.cs b/src/NetMQ/BufferPool.cs index 472df15cf..2150a6373 100644 --- a/src/NetMQ/BufferPool.cs +++ b/src/NetMQ/BufferPool.cs @@ -2,6 +2,7 @@ #if ! FLAVOR_MINIMAL using System.ServiceModel.Channels; #endif +using System.Collections.Generic; using System.Threading; using System.Buffers; @@ -34,6 +35,7 @@ public interface IBufferPool : IDisposable public class ArrayBufferPool : IBufferPool { private readonly ArrayPool m_arrayPool; + private readonly HashSet m_onLoan; /// /// Create a new ArrayBufferPool with the specified maximum buffer pool size @@ -45,7 +47,8 @@ public class ArrayBufferPool : IBufferPool /// Either maxBufferPoolSize or maxBufferSize was less than zero. public ArrayBufferPool(long maxBufferPoolSize, int maxBufferSize) { - m_arrayPool = ArrayPool.Create(maxBufferSize, (int) maxBufferPoolSize); + m_arrayPool = ArrayPool.Create(maxBufferSize, (int) maxBufferPoolSize); + m_onLoan = new HashSet(); } /// @@ -56,7 +59,9 @@ public ArrayBufferPool(long maxBufferPoolSize, int maxBufferSize) /// size cannot be less than zero public byte[] Take(int size) { - return m_arrayPool.Rent(size); + var loaner = m_arrayPool.Rent(size); + m_onLoan.Add(loaner); + return loaner; } /// @@ -67,7 +72,11 @@ public byte[] Take(int size) /// the buffer reference cannot be null public void Return(byte[] buffer) { - m_arrayPool.Return(buffer); + if (m_onLoan.Remove(buffer)) + m_arrayPool.Return(buffer); + else + throw new ArgumentException("Buffer returned to pool was not rented.", + nameof(buffer)); } /// @@ -87,7 +96,9 @@ protected virtual void Dispose(bool disposing) { if (!disposing) return; - // We don't have a means of clearing ArrayPool like BufferManager does. + foreach (var loaner in m_onLoan) + Return(loaner); + m_onLoan.Clear(); } } From 75f916a8336ec5c80a0bd11946ca6aa0820fa6e1 Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:33:09 -0400 Subject: [PATCH 5/9] doc: Note pool size does not constrain rent size. --- src/NetMQ.Tests/BufferPoolTests.cs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/NetMQ.Tests/BufferPoolTests.cs b/src/NetMQ.Tests/BufferPoolTests.cs index 3a8e820f1..1902d7868 100644 --- a/src/NetMQ.Tests/BufferPoolTests.cs +++ b/src/NetMQ.Tests/BufferPoolTests.cs @@ -1,3 +1,6 @@ +// See BufferPool.cs for explanation of USE_SERVICE_MODEL define. + +// #define USE_SERVICE_MODEL using System; using Xunit; @@ -8,7 +11,7 @@ public abstract class BufferPoolTests protected int maxBufferSize = 2; protected long maxBufferPoolSize = 100L; protected IBufferPool pool; - + [Theory] [InlineData(500)] [InlineData(50000)] @@ -16,11 +19,15 @@ public abstract class BufferPoolTests [InlineData(500000000)] public void Rent(int size) { + /* The pool sizes were chosen to be very small. It's clear they do + * not constitute any bounds on the array sizes that can be + * requested. BufferManagerBufferPool has the same behavior + * though. */ var array = pool.Take(size); Assert.Equal(size, array.Length); } - // This never happens. + // I was not able to provoke this to happen. Maybe with a long. // [Fact] // public void RentTooBigBuffer() // { @@ -29,14 +36,14 @@ public void Rent(int size) [Fact] public void Return() { - var array = pool.Take(10); - pool.Return(array); + var array = pool.Take(10); + pool.Return(array); } [Fact] public void ReturnUnknown() { - var array = new byte[100]; - Assert.Throws(() => pool.Return(array)); + var array = new byte[100]; + Assert.Throws(() => pool.Return(array)); } } @@ -44,21 +51,17 @@ public class ArrayBufferPoolTests : BufferPoolTests { public ArrayBufferPoolTests() { - /* The sizes were chosen to be very small. It's not clear they - * constitute any bounds on the array sizes that can be - * requested. */ pool = new ArrayBufferPool(maxBufferPoolSize, maxBufferSize); } } +#if USE_SERVICE_MODEL public class BufferManagerBufferPoolTests : BufferPoolTests { public BufferManagerBufferPoolTests() { - /* The sizes were chosen to be very small. It's not clear they - * constitute any bounds on the array sizes that can be - * requested. */ pool = new BufferManagerBufferPool(maxBufferPoolSize, maxBufferSize); } } +#endif } From 7c8512ceb27af1ea64dcc51d44dee67317dce008 Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:35:35 -0400 Subject: [PATCH 6/9] chore: Remove build Flavor parameter. --- src/NetMQ/NetMQ.csproj | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/NetMQ/NetMQ.csproj b/src/NetMQ/NetMQ.csproj index e85951b56..8261ecd25 100644 --- a/src/NetMQ/NetMQ.csproj +++ b/src/NetMQ/NetMQ.csproj @@ -22,21 +22,8 @@ true snupkg true - - Standard - - - - $(DefineConstants);FLAVOR_MINIMAL - - true From e41293015e249c678cb0eddc7c38c1dbe5baee7f Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:36:14 -0400 Subject: [PATCH 7/9] chore: Remove System.ServiceModel.Primitives ref. Subsitute with reference to System.Buffers. --- src/NetMQ/NetMQ.csproj | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/NetMQ/NetMQ.csproj b/src/NetMQ/NetMQ.csproj index 8261ecd25..f06e48478 100644 --- a/src/NetMQ/NetMQ.csproj +++ b/src/NetMQ/NetMQ.csproj @@ -37,16 +37,15 @@ + - - From 5a7e7b7a0eaa1520a0e7480f1646cca84da68daa Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:37:16 -0400 Subject: [PATCH 8/9] feature: Add USE_SERVICE_MODEL define for testing. I expect the BufferMangerBufferPool implementation will go away but before that happens I wanted the tests for each implementation to remain accessible. If you define USE_SERVICE_MODEL, it will build the old pool implementation and use it. And it'll also run tests against the old and new pool. --- src/NetMQ/BufferPool.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/NetMQ/BufferPool.cs b/src/NetMQ/BufferPool.cs index 2150a6373..ccc40369f 100644 --- a/src/NetMQ/BufferPool.cs +++ b/src/NetMQ/BufferPool.cs @@ -1,5 +1,10 @@ +// USE_SERVICE_MODEL requires the System.ServiceModel.Primitives package. +// It has been substituted by the System.Buffers package. This define remains +// merely to be able to exercise tests against both IBufferPool implementations. + +// #define USE_SERVICE_MODEL using System; -#if ! FLAVOR_MINIMAL +#if USE_SERVICE_MODEL using System.ServiceModel.Channels; #endif using System.Collections.Generic; @@ -102,7 +107,7 @@ protected virtual void Dispose(bool disposing) } } -#if ! FLAVOR_MINIMAL +#if USE_SERVICE_MODEL /// /// This implementation of uses WCF's /// class to manage a pool of buffers. @@ -252,10 +257,10 @@ public static void SetGCBufferPool() /// Either maxBufferPoolSize or maxBufferSize was less than zero. public static void SetBufferManagerBufferPool(long maxBufferPoolSize, int maxBufferSize) { -#if FLAVOR_MINIMAL - SetCustomBufferPool(new ArrayBufferPool(maxBufferPoolSize, maxBufferSize)); -#else +#if USE_SERVICE_MODEL SetCustomBufferPool(new BufferManagerBufferPool(maxBufferPoolSize, maxBufferSize)); +#else + SetCustomBufferPool(new ArrayBufferPool(maxBufferPoolSize, maxBufferSize)); #endif } From ceef6b9a93b6a6caf6c623df0608e21784878f84 Mon Sep 17 00:00:00 2001 From: Shane Celis Date: Wed, 11 May 2022 05:57:38 -0400 Subject: [PATCH 9/9] fix: Silence non-nullable warning in pool tests. --- src/NetMQ.Tests/BufferPoolTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/NetMQ.Tests/BufferPoolTests.cs b/src/NetMQ.Tests/BufferPoolTests.cs index 1902d7868..3ece815c1 100644 --- a/src/NetMQ.Tests/BufferPoolTests.cs +++ b/src/NetMQ.Tests/BufferPoolTests.cs @@ -10,7 +10,10 @@ public abstract class BufferPoolTests { protected int maxBufferSize = 2; protected long maxBufferPoolSize = 100L; + // Silence the non-nullable warning. +#pragma warning disable 8618 protected IBufferPool pool; +#pragma warning restore 8618 [Theory] [InlineData(500)]