From 4fea492b973080674ec7d38cf231a38acc3ee10a Mon Sep 17 00:00:00 2001 From: Aleksei Smirnov Date: Thu, 25 May 2023 13:24:40 +0300 Subject: [PATCH 1/3] Fix error with allocating more than MaxCapacity of Byte Memory Buffer --- .../PrimitiveColumnContainer.cs | 5 +++-- .../ReadOnlyDataFrameBuffer.cs | 5 ++++- test/Microsoft.Data.Analysis.Tests/BufferTests.cs | 12 ++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.Analysis/PrimitiveColumnContainer.cs b/src/Microsoft.Data.Analysis/PrimitiveColumnContainer.cs index 830440445e..0300b7ac85 100644 --- a/src/Microsoft.Data.Analysis/PrimitiveColumnContainer.cs +++ b/src/Microsoft.Data.Analysis/PrimitiveColumnContainer.cs @@ -181,7 +181,9 @@ public void AppendMany(T? value, long count) } DataFrameBuffer mutableLastBuffer = Buffers.GetOrCreateMutable(Buffers.Count - 1); - int allocatable = (int)Math.Min(remaining, ReadOnlyDataFrameBuffer.MaxCapacity); + + //Calculate how many values we can additionaly allocate and not exceed the MaxCapacity + int allocatable = (int)Math.Min(remaining, ReadOnlyDataFrameBuffer.MaxCapacity - mutableLastBuffer.Length); mutableLastBuffer.EnsureCapacity(allocatable); DataFrameBuffer lastNullBitMapBuffer = NullBitMapBuffers.GetOrCreateMutable(NullBitMapBuffers.Count - 1); @@ -205,7 +207,6 @@ public void AppendMany(T? value, long count) _modifyNullCountWhileIndexing = true; } - remaining -= allocatable; } } diff --git a/src/Microsoft.Data.Analysis/ReadOnlyDataFrameBuffer.cs b/src/Microsoft.Data.Analysis/ReadOnlyDataFrameBuffer.cs index 2bc41ebe51..2f08a18b5f 100644 --- a/src/Microsoft.Data.Analysis/ReadOnlyDataFrameBuffer.cs +++ b/src/Microsoft.Data.Analysis/ReadOnlyDataFrameBuffer.cs @@ -36,8 +36,11 @@ public ReadOnlyMemory RawReadOnlyMemory protected int Capacity => ReadOnlyBuffer.Length / Size; + //The maximum size in any single dimension for byte array is 0x7FFFFFc7 - 2147483591 + //See https://learn.microsoft.com/en-us/dotnet/framework/configure-apps/file-schema/runtime/gcallowverylargeobjects-element + public const int MaxCapacityInBytes = 2147483591; - public static int MaxCapacity => Int32.MaxValue / Size; + public static int MaxCapacity => MaxCapacityInBytes / Size; public ReadOnlySpan ReadOnlySpan { diff --git a/test/Microsoft.Data.Analysis.Tests/BufferTests.cs b/test/Microsoft.Data.Analysis.Tests/BufferTests.cs index 5f482955f6..75973200ed 100644 --- a/test/Microsoft.Data.Analysis.Tests/BufferTests.cs +++ b/test/Microsoft.Data.Analysis.Tests/BufferTests.cs @@ -133,6 +133,18 @@ public void TestAppendMany() Assert.False(intColumn.IsValid(7)); } + [Fact] + public void TestAppendMany_SizeMoreThanMaxBufferCapacity() + { + const int MaxCapacityInBytes = 2147483591; + + //Check appending values with extending column size over MaxCapacity of ReadOnlyDataFrameBuffer + PrimitiveDataFrameColumn intColumn = new PrimitiveDataFrameColumn("Byte1", MaxCapacityInBytes - 5); + intColumn.AppendMany(5, 10); + + Assert.Equal(MaxCapacityInBytes + 5, intColumn.Length); + } + [Fact] public void TestBasicArrowStringColumn() { From f6d402707606c66493e41611ce2b6f8372a743c4 Mon Sep 17 00:00:00 2001 From: Aleksei Smirnov Date: Thu, 25 May 2023 22:53:40 +0300 Subject: [PATCH 2/3] Remove Unit test as it consumes too much memory --- test/Microsoft.Data.Analysis.Tests/BufferTests.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/Microsoft.Data.Analysis.Tests/BufferTests.cs b/test/Microsoft.Data.Analysis.Tests/BufferTests.cs index 75973200ed..5f482955f6 100644 --- a/test/Microsoft.Data.Analysis.Tests/BufferTests.cs +++ b/test/Microsoft.Data.Analysis.Tests/BufferTests.cs @@ -133,18 +133,6 @@ public void TestAppendMany() Assert.False(intColumn.IsValid(7)); } - [Fact] - public void TestAppendMany_SizeMoreThanMaxBufferCapacity() - { - const int MaxCapacityInBytes = 2147483591; - - //Check appending values with extending column size over MaxCapacity of ReadOnlyDataFrameBuffer - PrimitiveDataFrameColumn intColumn = new PrimitiveDataFrameColumn("Byte1", MaxCapacityInBytes - 5); - intColumn.AppendMany(5, 10); - - Assert.Equal(MaxCapacityInBytes + 5, intColumn.Length); - } - [Fact] public void TestBasicArrowStringColumn() { From f23f4bf31bd6ca12d25793ef49362e5500245189 Mon Sep 17 00:00:00 2001 From: Aleksei Smirnov Date: Fri, 26 May 2023 00:58:48 +0300 Subject: [PATCH 3/3] Fix issue with increasing buffer capacity over limit when double it size --- .../DataFrameBuffer.cs | 5 ++++- .../BufferTests.cs | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.Analysis/DataFrameBuffer.cs b/src/Microsoft.Data.Analysis/DataFrameBuffer.cs index 352b853ddc..97778642df 100644 --- a/src/Microsoft.Data.Analysis/DataFrameBuffer.cs +++ b/src/Microsoft.Data.Analysis/DataFrameBuffer.cs @@ -66,7 +66,10 @@ public void EnsureCapacity(int numberOfValues) if (newLength > Capacity) { - var newCapacity = Math.Max(newLength * Size, ReadOnlyBuffer.Length * 2); + //Double buffer size, but not higher than MaxByteCapacity + var doubledSize = (int)Math.Min((long)ReadOnlyBuffer.Length * 2, MaxCapacityInBytes); + var newCapacity = Math.Max(newLength * Size, doubledSize); + var memory = new Memory(new byte[newCapacity]); _memory.CopyTo(memory); _memory = memory; diff --git a/test/Microsoft.Data.Analysis.Tests/BufferTests.cs b/test/Microsoft.Data.Analysis.Tests/BufferTests.cs index 5f482955f6..830085e626 100644 --- a/test/Microsoft.Data.Analysis.Tests/BufferTests.cs +++ b/test/Microsoft.Data.Analysis.Tests/BufferTests.cs @@ -188,6 +188,28 @@ public void TestArrowStringColumnClone() Assert.Null(clone[i]); } + /* Don't run tests during build as they fail, because build if build machine doesn't have enought memory + [Fact] + public void TestAppend_SizeMoreThanMaxBufferCapacity() + { + //Check appending value, than can increase buffer size over MaxCapacity (default strategy is to double buffer capacity) + PrimitiveDataFrameColumn intColumn = new PrimitiveDataFrameColumn("Byte1", int.MaxValue / 2 - 1); + intColumn.Append(10); + } + + [Fact] + public void TestAppendMany_SizeMoreThanMaxBufferCapacity() + { + const int MaxCapacityInBytes = 2147483591; + + //Check appending values with extending column size over MaxCapacity of ReadOnlyDataFrameBuffer + PrimitiveDataFrameColumn intColumn = new PrimitiveDataFrameColumn("Byte1", MaxCapacityInBytes - 5); + intColumn.AppendMany(5, 10); + + Assert.Equal(MaxCapacityInBytes + 5, intColumn.Length); + } + */ + //#if !NETFRAMEWORK // https://github.com/dotnet/corefxlab/issues/2796 // [Fact] // public void TestPrimitiveColumnGetReadOnlyBuffers()