From 663b7e630a886542ded313beeac83ba359aa6f0f Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 2 Jan 2018 04:09:27 +0000 Subject: [PATCH 1/4] Stream: stackalloc'd Span rather than byte array --- .../System/IO/PinnedBufferMemoryStream.cs | 8 +------- src/mscorlib/src/System/IO/Stream.cs | 18 ++++++++---------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs b/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs index 2bd1ef6b9537..a401c853f491 100644 --- a/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs +++ b/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs @@ -30,18 +30,12 @@ internal PinnedBufferMemoryStream(byte[] array) Debug.Assert(array != null, "Array can't be null"); int len = array.Length; - // Handle 0 length byte arrays specially. - if (len == 0) - { - array = new byte[1]; - len = 0; - } _array = array; _pinningHandle = GCHandle.Alloc(array, GCHandleType.Pinned); // Now the byte[] is pinned for the lifetime of this instance. // But I also need to get a pointer to that block of memory... - fixed (byte* ptr = &_array[0]) + fixed (byte* ptr = &MemoryMarshal.GetReference((Span)array)) Initialize(ptr, len, len, FileAccess.Read); } diff --git a/src/mscorlib/src/System/IO/Stream.cs b/src/mscorlib/src/System/IO/Stream.cs index d9ed08f737e0..8a0759e57cce 100644 --- a/src/mscorlib/src/System/IO/Stream.cs +++ b/src/mscorlib/src/System/IO/Stream.cs @@ -767,17 +767,16 @@ public virtual int Read(Span destination) // Reads one byte from the stream by calling Read(byte[], int, int). // Will return an unsigned byte cast to an int or -1 on end of stream. - // This implementation does not perform well because it allocates a new - // byte[] each time you call it, and should be overridden by any + // This implementation should be overridden by any // subclass that maintains an internal buffer. Then, it can help perf // significantly for people who are reading one byte at a time. public virtual int ReadByte() { - byte[] oneByteArray = new byte[1]; - int r = Read(oneByteArray, 0, 1); + Span bytes = stackalloc byte[1]; + int r = Read(bytes); if (r == 0) return -1; - return oneByteArray[0]; + return bytes[0]; } public abstract void Write(byte[] buffer, int offset, int count); @@ -794,15 +793,14 @@ public virtual void Write(ReadOnlySpan source) } // Writes one byte from the stream by calling Write(byte[], int, int). - // This implementation does not perform well because it allocates a new - // byte[] each time you call it, and should be overridden by any + // This implementation should be overridden by any // subclass that maintains an internal buffer. Then, it can help perf // significantly for people who are writing one byte at a time. public virtual void WriteByte(byte value) { - byte[] oneByteArray = new byte[1]; - oneByteArray[0] = value; - Write(oneByteArray, 0, 1); + Span bytes = stackalloc byte[1]; + bytes[0] = value; + Write(bytes); } public static Stream Synchronized(Stream stream) From 679ed5963ad3dbda2333a46b9a48e1b061d92927 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 2 Jan 2018 04:10:19 +0000 Subject: [PATCH 2/4] StubHelpers ConvertToManaged use stackalloc'd Span --- src/mscorlib/src/System/StubHelpers.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mscorlib/src/System/StubHelpers.cs b/src/mscorlib/src/System/StubHelpers.cs index bf19c7fa3401..90cf92552f96 100644 --- a/src/mscorlib/src/System/StubHelpers.cs +++ b/src/mscorlib/src/System/StubHelpers.cs @@ -46,7 +46,8 @@ unsafe static internal byte ConvertToNative(char managedChar, bool fBestFit, boo static internal char ConvertToManaged(byte nativeChar) { - byte[] bytes = new byte[1] { nativeChar }; + Span bytes = stackalloc byte[1]; + bytes[0] = nativeChar; string str = Encoding.Default.GetString(bytes); return str[0]; } From fb4e6e231dfbfaca223d1a3c97a07d3b64df4375 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Wed, 3 Jan 2018 03:25:23 +0000 Subject: [PATCH 3/4] feedback --- .../System/IO/PinnedBufferMemoryStream.cs | 3 +-- src/mscorlib/src/System/IO/Stream.cs | 18 ++++++++++-------- src/mscorlib/src/System/StubHelpers.cs | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs b/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs index a401c853f491..dfcc05d066f6 100644 --- a/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs +++ b/src/mscorlib/shared/System/IO/PinnedBufferMemoryStream.cs @@ -29,12 +29,11 @@ internal PinnedBufferMemoryStream(byte[] array) { Debug.Assert(array != null, "Array can't be null"); - int len = array.Length; - _array = array; _pinningHandle = GCHandle.Alloc(array, GCHandleType.Pinned); // Now the byte[] is pinned for the lifetime of this instance. // But I also need to get a pointer to that block of memory... + int len = array.Length; fixed (byte* ptr = &MemoryMarshal.GetReference((Span)array)) Initialize(ptr, len, len, FileAccess.Read); } diff --git a/src/mscorlib/src/System/IO/Stream.cs b/src/mscorlib/src/System/IO/Stream.cs index 8a0759e57cce..d9ed08f737e0 100644 --- a/src/mscorlib/src/System/IO/Stream.cs +++ b/src/mscorlib/src/System/IO/Stream.cs @@ -767,16 +767,17 @@ public virtual int Read(Span destination) // Reads one byte from the stream by calling Read(byte[], int, int). // Will return an unsigned byte cast to an int or -1 on end of stream. - // This implementation should be overridden by any + // This implementation does not perform well because it allocates a new + // byte[] each time you call it, and should be overridden by any // subclass that maintains an internal buffer. Then, it can help perf // significantly for people who are reading one byte at a time. public virtual int ReadByte() { - Span bytes = stackalloc byte[1]; - int r = Read(bytes); + byte[] oneByteArray = new byte[1]; + int r = Read(oneByteArray, 0, 1); if (r == 0) return -1; - return bytes[0]; + return oneByteArray[0]; } public abstract void Write(byte[] buffer, int offset, int count); @@ -793,14 +794,15 @@ public virtual void Write(ReadOnlySpan source) } // Writes one byte from the stream by calling Write(byte[], int, int). - // This implementation should be overridden by any + // This implementation does not perform well because it allocates a new + // byte[] each time you call it, and should be overridden by any // subclass that maintains an internal buffer. Then, it can help perf // significantly for people who are writing one byte at a time. public virtual void WriteByte(byte value) { - Span bytes = stackalloc byte[1]; - bytes[0] = value; - Write(bytes); + byte[] oneByteArray = new byte[1]; + oneByteArray[0] = value; + Write(oneByteArray, 0, 1); } public static Stream Synchronized(Stream stream) diff --git a/src/mscorlib/src/System/StubHelpers.cs b/src/mscorlib/src/System/StubHelpers.cs index 90cf92552f96..ae614d87ac4b 100644 --- a/src/mscorlib/src/System/StubHelpers.cs +++ b/src/mscorlib/src/System/StubHelpers.cs @@ -46,7 +46,7 @@ unsafe static internal byte ConvertToNative(char managedChar, bool fBestFit, boo static internal char ConvertToManaged(byte nativeChar) { - Span bytes = stackalloc byte[1]; + Span bytes = new Span(ref nativeChar, 1); bytes[0] = nativeChar; string str = Encoding.Default.GetString(bytes); return str[0]; From 6cd4db20c95fae881d69de6046b4d6bbc5e43609 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Wed, 3 Jan 2018 11:27:09 +0000 Subject: [PATCH 4/4] remove redundant assignment --- src/mscorlib/src/System/StubHelpers.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mscorlib/src/System/StubHelpers.cs b/src/mscorlib/src/System/StubHelpers.cs index ae614d87ac4b..fb196041ad46 100644 --- a/src/mscorlib/src/System/StubHelpers.cs +++ b/src/mscorlib/src/System/StubHelpers.cs @@ -47,7 +47,6 @@ unsafe static internal byte ConvertToNative(char managedChar, bool fBestFit, boo static internal char ConvertToManaged(byte nativeChar) { Span bytes = new Span(ref nativeChar, 1); - bytes[0] = nativeChar; string str = Encoding.Default.GetString(bytes); return str[0]; }