Skip to content

Commit

Permalink
Add TryGetBuffer, remove buggy bounds check
Browse files Browse the repository at this point in the history
Remove superfluous bounds checking. Add implementation of TryGetBuffer.
  • Loading branch information
Ben Watson committed May 17, 2019
1 parent 0184606 commit 927e173
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 28 deletions.
29 changes: 19 additions & 10 deletions UnitTests/Tests.cs
Expand Up @@ -337,7 +337,7 @@ public void GettingBlockAdjustsFreeAndInUseSize()
}
#endregion

#region GetBuffer Tests
#region GetBuffer/TryGetBuffer Tests
[Test]
public void GetBufferReturnsSingleBlockForBlockSize()
{
Expand All @@ -347,6 +347,7 @@ public void GetBufferReturnsSingleBlockForBlockSize()
stream.Write(buffer, 0, buffer.Length);
var returnedBuffer = stream.GetBuffer();
Assert.That(returnedBuffer.Length, Is.EqualTo(stream.MemoryManager.BlockSize));
RMSAssert.TryGetBufferEqualToGetBuffer(stream);
}

[Test]
Expand All @@ -358,6 +359,7 @@ public void GetBufferReturnsSingleBlockForLessThanBlockSize()
stream.Write(buffer, 0, buffer.Length);
var returnedBuffer = stream.GetBuffer();
Assert.That(returnedBuffer.Length, Is.EqualTo(stream.MemoryManager.BlockSize));
RMSAssert.TryGetBufferEqualToGetBuffer(stream);
}

[Test]
Expand All @@ -369,6 +371,7 @@ public void GetBufferReturnsLargeBufferForMoreThanBlockSize()
stream.Write(buffer, 0, buffer.Length);
var returnedBuffer = stream.GetBuffer();
Assert.That(returnedBuffer.Length, Is.EqualTo(stream.MemoryManager.LargeBufferMultiple));
RMSAssert.TryGetBufferEqualToGetBuffer(stream);
}

[Test]
Expand All @@ -380,6 +383,7 @@ public void GetBufferReturnsSameLarge()
var returnedBuffer = stream.GetBuffer();
var returnedBuffer2 = stream.GetBuffer();
Assert.That(returnedBuffer, Is.SameAs(returnedBuffer2));
RMSAssert.TryGetBufferEqualToGetBuffer(stream);
}

[Test]
Expand All @@ -392,6 +396,7 @@ public void GetBufferAdjustsLargePoolFreeSize()
stream.Write(buffer, 0, buffer.Length);

var newBuffer = stream.GetBuffer();
RMSAssert.TryGetBufferEqualToGetBuffer(stream);

stream.Dispose();

Expand All @@ -403,6 +408,7 @@ public void GetBufferAdjustsLargePoolFreeSize()
var newBuffer2 = newStream.GetBuffer();
Assert.That(newBuffer2.Length, Is.EqualTo(newBuffer.Length));
Assert.That(memMgr.LargePoolFreeSize, Is.EqualTo(0));
RMSAssert.TryGetBufferEqualToGetBuffer(newStream);
}

[Test]
Expand All @@ -422,6 +428,7 @@ public void CallingWriteAfterLargeGetBufferDoesNotLoseData()
Assert.That(buffer[stream.MemoryManager.BlockSize], Is.EqualTo(13));
RMSAssert.BuffersAreEqual(buffer, stream.MemoryManager.BlockSize + 1, bytesToWrite, 0, bytesToWrite.Length);
Assert.That(stream.Position, Is.EqualTo(stream.MemoryManager.BlockSize + 1 + bytesToWrite.Length));
RMSAssert.TryGetBufferEqualToGetBuffer(stream);
}

[Test]
Expand All @@ -440,6 +447,7 @@ public void CallingWriteByteAfterLargeGetBufferDoesNotLoseData()
Assert.That(buffer[stream.MemoryManager.BlockSize], Is.EqualTo(13));
Assert.That(buffer[stream.MemoryManager.BlockSize + 1], Is.EqualTo(14));
Assert.That(stream.Position, Is.EqualTo(stream.MemoryManager.BlockSize + 2));
RMSAssert.TryGetBufferEqualToGetBuffer(stream);
}

[Test]
Expand Down Expand Up @@ -2312,15 +2320,6 @@ protected virtual bool useExponentialLargeBuffer
get { return false; }
}

/*
* TODO: clocke to release logging libraries to enable some tests.
[TestFixtureSetUp]
public void Setup()
{
LogManager.Start();
}
*/

protected static class RMSAssert
{
/// <summary>
Expand Down Expand Up @@ -2371,6 +2370,16 @@ internal static void BuffersAreEqual(byte[] buffer1, byte[] buffer2, int count)
Assert.That(rate, Is.AtMost(tolerance), "Too many errors. Buffers can differ to a tolerance of {0:F4}",
tolerance);
}

internal static void TryGetBufferEqualToGetBuffer(RecyclableMemoryStream stream)
{
var buffer = stream.GetBuffer();
ArraySegment<byte> segment;
Assert.That(stream.TryGetBuffer(out segment), Is.True);
Assert.That(segment.Offset, Is.Zero);
Assert.That(segment.Count, Is.EqualTo(stream.Length));
Assert.That(segment.Array, Is.SameAs(buffer));
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.IO.RecyclableMemoryStream.csproj
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard1.4;netstandard2.1;net40;net45;netcoreapp2.1</TargetFrameworks>
<TargetFrameworks>netstandard1.4;netstandard2.1;net40;net45;net46;netcoreapp2.1</TargetFrameworks>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
</PropertyGroup>
<!-- for assembly signing we use a magic variable coupled with a special build definition which skips UTs -->
Expand Down
39 changes: 22 additions & 17 deletions src/RecyclableMemoryStream.cs
Expand Up @@ -452,6 +452,25 @@ public override byte[] GetBuffer()
return this.largeBuffer;
}

/// <summary>
/// Returns an ArraySegment that wraps a single buffer containing the contents of the stream.
/// </summary>
/// <param name="buffer">An ArraySegment containing a reference to the underlying bytes.</param>
/// <returns>Always returns true.</returns>
/// <remarks>GetBuffer has no failure modes (it always returns something, even if it's an empty buffer), therefore this method
/// always returns a valid ArraySegment to the same buffer returned by GetBuffer.</remarks>
#if NET40 || NET45
public bool TryGetBuffer(out ArraySegment<byte> buffer)
#else
public override bool TryGetBuffer(out ArraySegment<byte> buffer)
#endif
{
this.CheckDisposed();
buffer = new ArraySegment<byte>(this.GetBuffer(), 0, (int)this.Length);
// GetBuffer has no failure modes, so this should always succeed
return true;
}

/// <summary>
/// Returns a new array with a copy of the buffer's contents. You should almost certainly be using GetBuffer combined with the Length to
/// access the bytes in this stream. Calling ToArray will destroy the benefits of pooled buffers, but it is included
Expand Down Expand Up @@ -601,13 +620,6 @@ public override void Write(byte[] buffer, int offset, int count)
throw new IOException("Maximum capacity exceeded");
}

long requiredBuffers = (end + blockSize - 1) / blockSize;

if (requiredBuffers * blockSize > MaxStreamLength)
{
throw new IOException("Maximum capacity exceeded");
}

this.EnsureCapacity((int)end);

if (this.largeBuffer == null)
Expand Down Expand Up @@ -659,13 +671,6 @@ public override void Write(ReadOnlySpan<byte> source)
throw new IOException("Maximum capacity exceeded");
}

long requiredBuffers = (end + blockSize - 1) / blockSize;

if (requiredBuffers * blockSize > MaxStreamLength)
{
throw new IOException("Maximum capacity exceeded");
}

this.EnsureCapacity((int)end);

if (this.largeBuffer == null)
Expand Down Expand Up @@ -850,9 +855,9 @@ public override void WriteTo(Stream stream)
stream.Write(this.largeBuffer, 0, this.length);
}
}
#endregion
#endregion

#region Helper Methods
#region Helper Methods
private bool Disposed => Interlocked.Read(ref this.disposedState) != 0;

private void CheckDisposed()
Expand Down Expand Up @@ -1005,6 +1010,6 @@ private void ReleaseLargeBuffer()

this.largeBuffer = null;
}
#endregion
#endregion
}
}

0 comments on commit 927e173

Please sign in to comment.