Skip to content

Commit

Permalink
Add missing calls to CheckDisposed in RecyclableMemoryStream. Integra…
Browse files Browse the repository at this point in the history 
…te pull request changes for computing block+offset atomically for performance improvements. Misc spell fixes and formatting fixes.
  • Loading branch information
@doubleyewdee
doubleyewdee committed Mar 23, 2015
1 parent d1a4f1b commit 56367f0
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 44 deletions.
4 changes: 2 additions & 2 deletions UnitTests/Tests.cs
View file
Expand Up @@ -1427,8 +1427,8 @@ public void DisposeReturnsLargeBuffer()
Assert.That(memMgr.LargePoolInUseSize, Is.EqualTo(0));
}

[Test, ExpectedException(typeof(InvalidOperationException))]
public void DisposeTwiceThrowsException()
[Test]
public void DisposeTwiceDoesNotThrowException()
{
var stream = this.GetDefaultStream();
stream.Dispose();
Expand Down
87 changes: 45 additions & 42 deletions src/RecyclableMemoryStream.cs
View file
Expand Up @@ -40,7 +40,7 @@ namespace Microsoft.IO
/// leads to continual memory growth as each stream approaches the maximum allowed size.
/// 3. Memory copying - Each time a MemoryStream grows, all the bytes are copied into new buffers.
/// This implementation only copies the bytes when GetBuffer is called.
/// 4. Memory fragmentation - By using homogenous buffer sizes, it ensures that blocks of memory
/// 4. Memory fragmentation - By using homogeneous buffer sizes, it ensures that blocks of memory
/// can be easily reused.
///
/// The stream is implemented on top of a series of uniformly-sized blocks. As the stream's length grows,
Expand Down Expand Up @@ -140,9 +140,8 @@ internal RecyclableMemoryStreamManager MemoryManager
/// </summary>
/// <param name="memoryManager">The memory manager</param>
public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager)
: this(memoryManager, null)
: this(memoryManager, null, 0, null)
{

}

/// <summary>
Expand All @@ -151,9 +150,8 @@ public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager)
/// <param name="memoryManager">The memory manager</param>
/// <param name="tag">A string identifying this stream for logging and debugging purposes</param>
public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager, string tag)
: this(memoryManager, tag, 0)
: this(memoryManager, tag, 0, null)
{

}

/// <summary>
Expand Down Expand Up @@ -208,7 +206,6 @@ public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager, strin
#endregion

#region Dispose and Finalize

~RecyclableMemoryStream()
{
this.Dispose(false);
Expand All @@ -231,7 +228,7 @@ protected override void Dispose(bool disposing)
}

Events.Write.MemoryStreamDoubleDispose(this.id, this.tag, this.allocationStack, this.disposeStack, doubleDisposeStack);
throw new InvalidOperationException("Cannot dispose of RecyclableMemoryStream twice");
return;
}

Events.Write.MemoryStreamDisposed(this.id, this.tag);
Expand Down Expand Up @@ -295,7 +292,6 @@ public override void Close()
{
this.Dispose(true);
}

#endregion

#region MemoryStream overrides
Expand Down Expand Up @@ -325,7 +321,11 @@ public override int Capacity
}
return 0;
}
set { this.EnsureCapacity(value); }
set
{
this.CheckDisposed();
this.EnsureCapacity(value);
}
}

private int length;
Expand Down Expand Up @@ -358,6 +358,7 @@ public override long Position
}
set
{
this.CheckDisposed();
if (value < 0)
{
throw new ArgumentOutOfRangeException("value", "value must be non-negative");
Expand All @@ -368,7 +369,7 @@ public override long Position
throw new ArgumentOutOfRangeException("value", "value cannot be more than " + MaxStreamLength);
}

this.position= (int)value;
this.position = (int)value;
}
}

Expand Down Expand Up @@ -430,7 +431,7 @@ public override byte[] GetBuffer()
// it's possible that people will manipulate the buffer directly
// and set the length afterward. Capacity sets the expectation
// for the size of the buffer.
var newBuffer = this.MemoryManager.GetLargeBuffer(this.Capacity, this.tag);
var newBuffer = this.memoryManager.GetLargeBuffer(this.Capacity, this.tag);

// InternalRead will check for existence of largeBuffer, so make sure we
// don't set it until after we've copied the data.
Expand Down Expand Up @@ -538,46 +539,42 @@ public override void Write(byte[] buffer, int offset, int count)
throw new ArgumentException("count must be greater than buffer.Length - offset");
}

int blockSize = this.memoryManager.BlockSize;
long end = this.Position + count;
// Check for overflow
if (this.Position + count > MaxStreamLength)
if (end > MaxStreamLength)
{
throw new IOException("Maximum capacity exceeded");
}

int end = (int)this.Position + count;

int blockSize = this.memoryManager.BlockSize;

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

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

EnsureCapacity(end);
this.EnsureCapacity((int)end);

if (this.largeBuffer == null)
{
int bytesRemaining = count;
int bytesWritten = 0;
int currentBlockIndex = this.OffsetToBlockIndex(this.position);

int blockOffset = this.OffsetToBlockOffset(this.position);
var blockAndOffset = this.GetBlockAndRelativeOffset(this.position);

while (bytesRemaining > 0)
{
byte[] currentBlock = this.blocks[currentBlockIndex];
int remainingInBlock = blockSize - blockOffset;
byte[] currentBlock = this.blocks[blockAndOffset.Block];
int remainingInBlock = blockSize - blockAndOffset.Offset;
int amountToWriteInBlock = Math.Min(remainingInBlock, bytesRemaining);

Buffer.BlockCopy(buffer, offset + bytesWritten, currentBlock, blockOffset, amountToWriteInBlock);
Buffer.BlockCopy(buffer, offset + bytesWritten, currentBlock, blockAndOffset.Offset, amountToWriteInBlock);

bytesRemaining -= amountToWriteInBlock;
bytesWritten += amountToWriteInBlock;

currentBlockIndex++;
blockOffset = 0;
++blockAndOffset.Block;
blockAndOffset.Offset = 0;
}
}
else
Expand All @@ -603,7 +600,7 @@ public override string ToString()
/// <exception cref="ObjectDisposedException">Object has been disposed</exception>
public override void WriteByte(byte value)
{
CheckDisposed();
this.CheckDisposed();
this.byteBuffer[0] = value;
this.Write(this.byteBuffer, 0, 1);
}
Expand All @@ -623,9 +620,8 @@ public override int ReadByte()
byte value = 0;
if (this.largeBuffer == null)
{
int block = OffsetToBlockIndex(this.position);
int blockOffset = OffsetToBlockOffset(this.position);
value = this.blocks[block][blockOffset];
var blockAndOffset = this.GetBlockAndRelativeOffset(this.position);
value = this.blocks[blockAndOffset.Block][blockAndOffset.Offset];
}
else
{
Expand Down Expand Up @@ -705,6 +701,7 @@ public override long Seek(long offset, SeekOrigin loc)
/// <remarks>Important: This does a synchronous write, which may not be desired in some situations</remarks>
public override void WriteTo(Stream stream)
{
this.CheckDisposed();
if (stream == null)
{
throw new ArgumentNullException("stream");
Expand Down Expand Up @@ -733,7 +730,6 @@ public override void WriteTo(Stream stream)
#endregion

#region Helper Methods

private void CheckDisposed()
{
if (this.disposed)
Expand All @@ -750,21 +746,20 @@ private int InternalRead(byte[] buffer, int offset, int count, int fromPosition)
}
if (this.largeBuffer == null)
{
int currentBlock = this.OffsetToBlockIndex(fromPosition);
var blockAndOffset = this.GetBlockAndRelativeOffset(fromPosition);
int bytesWritten = 0;
int bytesRemaining = Math.Min(count, this.length - fromPosition);
int blockOffset = this.OffsetToBlockOffset(fromPosition);

while (bytesRemaining > 0)
{
int amountToCopy = Math.Min(this.blocks[currentBlock].Length - blockOffset, bytesRemaining);
Buffer.BlockCopy(this.blocks[currentBlock], blockOffset, buffer, bytesWritten + offset, amountToCopy);
int amountToCopy = Math.Min(this.blocks[blockAndOffset.Block].Length - blockAndOffset.Offset, bytesRemaining);
Buffer.BlockCopy(this.blocks[blockAndOffset.Block], blockAndOffset.Offset, buffer, bytesWritten + offset, amountToCopy);

bytesWritten += amountToCopy;
bytesRemaining -= amountToCopy;

++currentBlock;
blockOffset = 0;
++blockAndOffset.Block;
blockAndOffset.Offset = 0;
}
return bytesWritten;
}
Expand All @@ -776,14 +771,22 @@ private int InternalRead(byte[] buffer, int offset, int count, int fromPosition)
}
}

private int OffsetToBlockIndex(int offset)
private struct BlockAndOffset
{
return offset / this.memoryManager.BlockSize;
public int Block;
public int Offset;

public BlockAndOffset(int block, int offset)
{
this.Block = block;
this.Offset = offset;
}
}

private int OffsetToBlockOffset(int offset)
private BlockAndOffset GetBlockAndRelativeOffset(int offset)
{
return offset % this.memoryManager.BlockSize;
var blockSize = this.memoryManager.BlockSize;
return new BlockAndOffset(offset / blockSize, offset % blockSize);
}

private void EnsureCapacity(int newCapacity)
Expand All @@ -808,7 +811,7 @@ private void EnsureCapacity(int newCapacity)
{
while (this.Capacity < newCapacity)
{
blocks.Add((this.MemoryManager.GetBlock()));
blocks.Add((this.memoryManager.GetBlock()));
}
}
}
Expand Down

0 comments on commit 56367f0

Please sign in to comment.