Override CopyTo in MemoryStream (dotnet/coreclr#7131)
authorJames Ko <jamesqko@gmail.com>
Thu, 15 Sep 2016 11:37:41 +0000 (07:37 -0400)
committerJan Vorlicek <janvorli@microsoft.com>
Thu, 15 Sep 2016 11:37:41 +0000 (13:37 +0200)
* Cache some virtual method calls in Stream.ValidateCopyToArguments
* Add override of Stream.CopyTo to MemoryStream

Commit migrated from https://github.com/dotnet/coreclr/commit/a62af3e824095dc11263f040705623f2e216165c

src/coreclr/src/mscorlib/src/System/IO/MemoryStream.cs
src/coreclr/src/mscorlib/src/System/IO/Stream.cs

index edb583b..a174621 100644 (file)
@@ -402,36 +402,46 @@ namespace System.IO {
             return _buffer[_position++];
         }
 
+        public override void CopyTo(Stream destination, int bufferSize)
+        {
+            // Since we did not originally override this method, validate the arguments
+            // the same way Stream does for back-compat.
+            ValidateCopyToArguments(destination, bufferSize);
 
-        public override Task CopyToAsync(Stream destination, Int32 bufferSize, CancellationToken cancellationToken) {
-
-            // This implementation offers beter performance compared to the base class version.
-
-            // The parameter checks must be in sync with the base version:
-            if (destination == null)
-                throw new ArgumentNullException("destination");
+            // If we have been inherited into a subclass, the following implementation could be incorrect
+            // since it does not call through to Read() which a subclass might have overridden.  
+            // To be safe we will only use this implementation in cases where we know it is safe to do so,
+            // and delegate to our base class (which will call into Read) when we are not sure.
+            if (GetType() != typeof(MemoryStream))
+            {
+                base.CopyTo(destination, bufferSize);
+                return;
+            }
             
-            if (bufferSize <= 0)
-                throw new ArgumentOutOfRangeException("bufferSize", Environment.GetResourceString("ArgumentOutOfRange_NeedPosNum"));
+            int originalPosition = _position;
 
-            if (!CanRead && !CanWrite)
-                throw new ObjectDisposedException(null, Environment.GetResourceString("ObjectDisposed_StreamClosed"));
+            // Seek to the end of the MemoryStream.
+            int remaining = InternalEmulateRead(_length - originalPosition);
 
-            if (!destination.CanRead && !destination.CanWrite)
-                throw new ObjectDisposedException("destination", Environment.GetResourceString("ObjectDisposed_StreamClosed"));
+            // If we were already at or past the end, there's no copying to do so just quit.
+            if (remaining > 0)
+            {
+                // Call Write() on the other Stream, using our internal buffer and avoiding any
+                // intermediary allocations.
+                destination.Write(_buffer, originalPosition, remaining);
+            }
+        }
 
-            if (!CanRead)
-                throw new NotSupportedException(Environment.GetResourceString("NotSupported_UnreadableStream"));
+        public override Task CopyToAsync(Stream destination, Int32 bufferSize, CancellationToken cancellationToken) {
 
-            if (!destination.CanWrite)
-                throw new NotSupportedException(Environment.GetResourceString("NotSupported_UnwritableStream"));
+            // This implementation offers beter performance compared to the base class version.
 
-            Contract.EndContractBlock();
+            ValidateCopyToArguments(destination, bufferSize);
 
             // If we have been inherited into a subclass, the following implementation could be incorrect
-            // since it does not call through to Read() or Write() which a subclass might have overriden.  
+            // since it does not call through to ReadAsync() which a subclass might have overridden.  
             // To be safe we will only use this implementation in cases where we know it is safe to do so,
-            // and delegate to our base class (which will call into Read/Write) when we are not sure.
+            // and delegate to our base class (which will call into ReadAsync) when we are not sure.
             if (this.GetType() != typeof(MemoryStream))
                 return base.CopyToAsync(destination, bufferSize, cancellationToken);
 
index 7b7b0d3..4ec0a31 100644 (file)
@@ -861,13 +861,18 @@ namespace System.IO {
                 throw new ArgumentNullException("destination");
             if (bufferSize <= 0)
                 throw new ArgumentOutOfRangeException("bufferSize", Environment.GetResourceString("ArgumentOutOfRange_NeedPosNum"));
-            if (!CanRead && !CanWrite)
+
+            // Cache some virtual method calls for better perf
+            bool canRead = CanRead;
+            bool destCanWrite = destination.CanWrite;
+
+            if (!canRead && !CanWrite)
                 throw new ObjectDisposedException(null, Environment.GetResourceString("ObjectDisposed_StreamClosed"));
-            if (!destination.CanRead && !destination.CanWrite)
+            if (!destination.CanRead && !destCanWrite)
                 throw new ObjectDisposedException("destination", Environment.GetResourceString("ObjectDisposed_StreamClosed"));
-            if (!CanRead)
+            if (!canRead)
                 throw new NotSupportedException(Environment.GetResourceString("NotSupported_UnreadableStream"));
-            if (!destination.CanWrite)
+            if (!destCanWrite)
                 throw new NotSupportedException(Environment.GetResourceString("NotSupported_UnwritableStream"));
             Contract.EndContractBlock();
         }