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.

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>

Author: stephentoub

src/Common/src/CoreLib/System/IO/StreamReader.cs

@@ -70,21 +70,21 @@ public class StreamReader : TextReader
         // 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 

src/Common/src/CoreLib/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 @@ public class StreamWriter : TextWriter
         // 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 @@ public override void Write(char value)
             }
         }
 
+        [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 @@ public override void Write(char[] buffer, int index, int count)
                 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 @@ public override void Write(ReadOnlySpan<char> buffer)
             }
         }
 
-        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 @@ private unsafe void WriteCore(ReadOnlySpan<char> buffer, bool autoFlush)
                 }
             }
 
-            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
             {