diff options
author | Stephen Toub <stoub@microsoft.com> | 2018-03-27 15:11:15 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-27 15:11:15 -0700 |
commit | 289ebe644ef88aa04b2bf509833689b2bf01485a (patch) | |
tree | a5487c855b0e19b8d04cbab3d596da6052540f7a | |
parent | 550f12783c9b4bbad82c86552c44d1ed29d75bfd (diff) | |
download | coreclr-289ebe644ef88aa04b2bf509833689b2bf01485a.tar.gz coreclr-289ebe644ef88aa04b2bf509833689b2bf01485a.tar.bz2 coreclr-289ebe644ef88aa04b2bf509833689b2bf01485a.zip |
Fix regression in StreamWriter.Write perf for small inputs (#17251)
* Streamline StreamWriter/Reader.CheckAsyncTaskInProgress and make it inlineable
* Re-inline StreamWriter.WriteSpan
Prior to my span changes, the code in WriteCore was manually inlined into each of its call sites. I'd refactored it out into a shared helper to reduce code duplication, but that introduced a regression for small inputs. I'm keeping the factoring but making it AggressiveInlining to avoid that regression while still keeping the C# code reuse.
* Use NoInlining on WriteSpan call sites
In case methods get devirtualized and JIT attempts to inline them and WriteSpan.
-rw-r--r-- | src/mscorlib/shared/System/IO/StreamReader.cs | 12 | ||||
-rw-r--r-- | src/mscorlib/shared/System/IO/StreamWriter.cs | 68 |
2 files changed, 45 insertions, 35 deletions
diff --git a/src/mscorlib/shared/System/IO/StreamReader.cs b/src/mscorlib/shared/System/IO/StreamReader.cs index 22ec6e645b..ea30541f5a 100644 --- a/src/mscorlib/shared/System/IO/StreamReader.cs +++ b/src/mscorlib/shared/System/IO/StreamReader.cs @@ -70,21 +70,21 @@ namespace System.IO // We don't guarantee thread safety on StreamReader, but we should at // least prevent users from trying to read anything while an Async // read from the same thread is in progress. - private volatile Task _asyncReadTask; + private Task _asyncReadTask = Task.CompletedTask; private void CheckAsyncTaskInProgress() { // We are not locking the access to _asyncReadTask because this is not meant to guarantee thread safety. // We are simply trying to deter calling any Read APIs while an async Read from the same thread is in progress. - - Task t = _asyncReadTask; - - if (t != null && !t.IsCompleted) + if (!_asyncReadTask.IsCompleted) { - throw new InvalidOperationException(SR.InvalidOperation_AsyncIOInProgress); + ThrowAsyncIOInProgress(); } } + private static void ThrowAsyncIOInProgress() => + throw new InvalidOperationException(SR.InvalidOperation_AsyncIOInProgress); + // StreamReader by default will ignore illegal UTF8 characters. We don't want to // throw here because we want to be able to read ill-formed data without choking. // The high level goal is to be tolerant of encoding errors when we read and very strict diff --git a/src/mscorlib/shared/System/IO/StreamWriter.cs b/src/mscorlib/shared/System/IO/StreamWriter.cs index a376244280..06d94d2385 100644 --- a/src/mscorlib/shared/System/IO/StreamWriter.cs +++ b/src/mscorlib/shared/System/IO/StreamWriter.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; using System.Threading; @@ -44,19 +45,21 @@ namespace System.IO // We don't guarantee thread safety on StreamWriter, but we should at // least prevent users from trying to write anything while an Async // write from the same thread is in progress. - private volatile Task _asyncWriteTask; + private Task _asyncWriteTask = Task.CompletedTask; private void CheckAsyncTaskInProgress() { // We are not locking the access to _asyncWriteTask because this is not meant to guarantee thread safety. // We are simply trying to deter calling any Write APIs while an async Write from the same thread is in progress. - - Task t = _asyncWriteTask; - - if (t != null && !t.IsCompleted) - throw new InvalidOperationException(SR.InvalidOperation_AsyncIOInProgress); + if (!_asyncWriteTask.IsCompleted) + { + ThrowAsyncIOInProgress(); + } } + private static void ThrowAsyncIOInProgress() => + throw new InvalidOperationException(SR.InvalidOperation_AsyncIOInProgress); + // The high level goal is to be tolerant of encoding errors when we read and very strict // when we write. Hence, default StreamWriter encoding will throw on encoding error. // Note: when StreamWriter throws on invalid encoding chars (for ex, high surrogate character @@ -323,14 +326,13 @@ namespace System.IO } } + [MethodImpl(MethodImplOptions.NoInlining)] // prevent WriteSpan from bloating call sites public override void Write(char[] buffer) { - if (buffer != null) - { - WriteCore(buffer, _autoFlush); - } + WriteSpan(buffer, appendNewLine: false); } + [MethodImpl(MethodImplOptions.NoInlining)] // prevent WriteSpan from bloating call sites public override void Write(char[] buffer, int index, int count) { if (buffer == null) @@ -350,14 +352,15 @@ namespace System.IO throw new ArgumentException(SR.Argument_InvalidOffLen); } - WriteCore(new ReadOnlySpan<char>(buffer, index, count), _autoFlush); + WriteSpan(buffer.AsSpan(index, count), appendNewLine: false); } + [MethodImpl(MethodImplOptions.NoInlining)] // prevent WriteSpan from bloating call sites public override void Write(ReadOnlySpan<char> buffer) { if (GetType() == typeof(StreamWriter)) { - WriteCore(buffer, _autoFlush); + WriteSpan(buffer, appendNewLine: false); } else { @@ -367,7 +370,8 @@ namespace System.IO } } - private unsafe void WriteCore(ReadOnlySpan<char> buffer, bool autoFlush) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private unsafe void WriteSpan(ReadOnlySpan<char> buffer, bool appendNewLine) { CheckAsyncTaskInProgress(); @@ -422,41 +426,47 @@ namespace System.IO } } - if (autoFlush) + if (appendNewLine) + { + char[] coreNewLine = CoreNewLine; + for (int i = 0; i < coreNewLine.Length; i++) // Generally 1 (\n) or 2 (\r\n) iterations + { + if (_charPos == _charLen) + { + Flush(false, false); + } + + _charBuffer[_charPos] = coreNewLine[i]; + _charPos++; + } + } + + if (_autoFlush) { Flush(true, false); } } + [MethodImpl(MethodImplOptions.NoInlining)] // prevent WriteSpan from bloating call sites public override void Write(string value) { - if (value != null) - { - WriteCore(value.AsSpan(), _autoFlush); - } + WriteSpan(value, appendNewLine: false); } - // - // Optimize the most commonly used WriteLine overload. This optimization is important for System.Console in particular - // because of it will make one WriteLine equal to one call to the OS instead of two in the common case. - // + [MethodImpl(MethodImplOptions.NoInlining)] // prevent WriteSpan from bloating call sites public override void WriteLine(string value) { CheckAsyncTaskInProgress(); - if (value != null) - { - WriteCore(value.AsSpan(), autoFlush: false); - } - WriteCore(new ReadOnlySpan<char>(CoreNewLine), autoFlush: true); + WriteSpan(value, appendNewLine: true); } + [MethodImpl(MethodImplOptions.NoInlining)] // prevent WriteSpan from bloating call sites public override void WriteLine(ReadOnlySpan<char> value) { if (GetType() == typeof(StreamWriter)) { CheckAsyncTaskInProgress(); - WriteCore(value, autoFlush: false); - WriteCore(new ReadOnlySpan<char>(CoreNewLine), autoFlush: true); + WriteSpan(value, appendNewLine: true); } else { |