From 35a7731be4a089da9310985755e5b02604701928 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 17 Apr 2024 17:20:33 -0700 Subject: [PATCH 1/4] Bypass SyncTextWriter on single-threaded wasm to avoid compiling synchronization code during startup when console is touched --- .../System.Console/src/System/Console.cs | 33 +++++++++++++------ .../System.Console/tests/SyncTextWriter.cs | 1 + .../System.Private.CoreLib.Shared.projitems | 2 ++ 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index d2db8f831d448..9feb62da665a8 100644 --- a/src/libraries/System.Console/src/System/Console.cs +++ b/src/libraries/System.Console/src/System/Console.cs @@ -235,16 +235,23 @@ static TextWriter EnsureInitialized() private static TextWriter CreateOutputWriter(Stream outputStream) { - return outputStream == Stream.Null ? - TextWriter.Null : - TextWriter.Synchronized(new StreamWriter( - stream: outputStream, - encoding: OutputEncoding.RemovePreamble(), // This ensures no prefix is written to the stream. - bufferSize: WriteBufferSize, - leaveOpen: true) - { - AutoFlush = true - }); + if (outputStream == Stream.Null) + return TextWriter.Null; + + StreamWriter writer = new StreamWriter( + stream: outputStream, + encoding: OutputEncoding.RemovePreamble(), // This ensures no prefix is written to the stream. + bufferSize: WriteBufferSize, + leaveOpen: true + ) { + AutoFlush = true + }; + +#if TARGET_BROWSER && !FEATURE_WASM_MANAGED_THREADS + return writer; +#else + return TextWriter.Synchronized(writer); +#endif } private static StrongBox? _isStdInRedirected; @@ -702,7 +709,10 @@ public static void SetOut(TextWriter newOut) // are nops. if (newOut != TextWriter.Null) { +#if TARGET_BROWSER && !FEATURE_WASM_MANAGED_THREADS +#else newOut = TextWriter.Synchronized(newOut); +#endif } lock (s_syncObject) @@ -719,7 +729,10 @@ public static void SetError(TextWriter newError) // Ensure all access to the writer is synchronized. See comment in SetOut. if (newError != TextWriter.Null) { +#if TARGET_BROWSER && !FEATURE_WASM_MANAGED_THREADS +#else newError = TextWriter.Synchronized(newError); +#endif } lock (s_syncObject) diff --git a/src/libraries/System.Console/tests/SyncTextWriter.cs b/src/libraries/System.Console/tests/SyncTextWriter.cs index 7d0a2f3016b64..f00dfa702b79b 100644 --- a/src/libraries/System.Console/tests/SyncTextWriter.cs +++ b/src/libraries/System.Console/tests/SyncTextWriter.cs @@ -13,6 +13,7 @@ public class SyncTextWriter { [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] public void SyncTextWriterLockedOnThis() { TextWriter oldWriter = Console.Out; diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 233d19f0d5b5f..1ed17ee58cde7 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -22,6 +22,8 @@ true true true + true + $(DefineConstants);FEATURE_WASM_MANAGED_THREADS $(DefineConstants);BIGENDIAN From 1216d845d2dc232ca3b6bdf1c64fccc26efb917b Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Mon, 22 Apr 2024 14:32:22 -0700 Subject: [PATCH 2/4] Address PR feedback --- .../System.Console/src/System/Console.cs | 33 ++++++------------- .../src/System/IO/TextWriter.cs | 4 +++ 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index 9feb62da665a8..d2db8f831d448 100644 --- a/src/libraries/System.Console/src/System/Console.cs +++ b/src/libraries/System.Console/src/System/Console.cs @@ -235,23 +235,16 @@ static TextWriter EnsureInitialized() private static TextWriter CreateOutputWriter(Stream outputStream) { - if (outputStream == Stream.Null) - return TextWriter.Null; - - StreamWriter writer = new StreamWriter( - stream: outputStream, - encoding: OutputEncoding.RemovePreamble(), // This ensures no prefix is written to the stream. - bufferSize: WriteBufferSize, - leaveOpen: true - ) { - AutoFlush = true - }; - -#if TARGET_BROWSER && !FEATURE_WASM_MANAGED_THREADS - return writer; -#else - return TextWriter.Synchronized(writer); -#endif + return outputStream == Stream.Null ? + TextWriter.Null : + TextWriter.Synchronized(new StreamWriter( + stream: outputStream, + encoding: OutputEncoding.RemovePreamble(), // This ensures no prefix is written to the stream. + bufferSize: WriteBufferSize, + leaveOpen: true) + { + AutoFlush = true + }); } private static StrongBox? _isStdInRedirected; @@ -709,10 +702,7 @@ public static void SetOut(TextWriter newOut) // are nops. if (newOut != TextWriter.Null) { -#if TARGET_BROWSER && !FEATURE_WASM_MANAGED_THREADS -#else newOut = TextWriter.Synchronized(newOut); -#endif } lock (s_syncObject) @@ -729,10 +719,7 @@ public static void SetError(TextWriter newError) // Ensure all access to the writer is synchronized. See comment in SetOut. if (newError != TextWriter.Null) { -#if TARGET_BROWSER && !FEATURE_WASM_MANAGED_THREADS -#else newError = TextWriter.Synchronized(newError); -#endif } lock (s_syncObject) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs b/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs index a6070683047a3..d949017809f82 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs @@ -759,7 +759,11 @@ public static TextWriter Synchronized(TextWriter writer) { ArgumentNullException.ThrowIfNull(writer); +#if !TARGET_BROWSER || FEATURE_WASM_MANAGED_THREADS return writer is SyncTextWriter ? writer : new SyncTextWriter(writer); +#else + return writer; +#endif } internal sealed class SyncTextWriter : TextWriter, IDisposable From 45af68198610c986299bd2e10d5438fda8457251 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 24 Apr 2024 10:51:02 -0700 Subject: [PATCH 3/4] Test fixes --- src/libraries/System.Console/tests/ReadAndWrite.cs | 4 +++- .../tests/System.IO.Tests/StreamReader/StreamReader.cs | 1 + .../tests/System.IO.Tests/StreamWriter/StreamWriter.cs | 1 + .../tests/System.IO.Tests/TextWriter/TextWriterTests.cs | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Console/tests/ReadAndWrite.cs b/src/libraries/System.Console/tests/ReadAndWrite.cs index d60fd6aa34cc0..913d85835dbb1 100644 --- a/src/libraries/System.Console/tests/ReadAndWrite.cs +++ b/src/libraries/System.Console/tests/ReadAndWrite.cs @@ -158,7 +158,9 @@ public static async Task OutWriteAndWriteLineOverloads() Console.SetOut(sw); TextWriter writer = Console.Out; Assert.NotNull(writer); - Assert.NotEqual(writer, sw); // the writer we provide gets wrapped + // Browser bypasses SyncTextWriter for faster startup + if (!OperatingSystem.IsBrowser()) + Assert.NotEqual(writer, sw); // the writer we provide gets wrapped // We just want to ensure none of these throw exceptions, we don't actually validate // what was written. diff --git a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs index bd455448efcca..bf50637d5f7ac 100644 --- a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs +++ b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs @@ -30,6 +30,7 @@ public void ObjectClosedReadLineBaseStream() } [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] public void Synchronized_NewObject() { using (Stream str = GetLargeStream()) diff --git a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs index 69edb13a8a1fc..eb4718aaf0f2e 100644 --- a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs +++ b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs @@ -8,6 +8,7 @@ namespace System.IO.Tests public partial class WriteTests { [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] public void Synchronized_NewObject() { using (Stream str = CreateStream()) diff --git a/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs b/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs index 2299dc86f567d..d021010e54d9d 100644 --- a/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs +++ b/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs @@ -691,6 +691,7 @@ public void DisposeAsync_ExceptionReturnedInTask() } [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] public async Task FlushAsync_Precanceled() { Assert.Equal(TaskStatus.RanToCompletion, TextWriter.Null.FlushAsync(new CancellationToken(true)).Status); From 86293a1f253e57b031a8c9d2e39d58810dcc8cc1 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 3 Jul 2024 18:41:07 -0700 Subject: [PATCH 4/4] Narrow test filtering to single-threaded --- src/libraries/System.Console/tests/SyncTextWriter.cs | 4 ++-- .../tests/System.IO.Tests/StreamReader/StreamReader.cs | 4 ++-- .../tests/System.IO.Tests/StreamWriter/StreamWriter.cs | 4 ++-- .../tests/System.IO.Tests/TextWriter/TextWriterTests.cs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Console/tests/SyncTextWriter.cs b/src/libraries/System.Console/tests/SyncTextWriter.cs index f00dfa702b79b..772e1e2b44a91 100644 --- a/src/libraries/System.Console/tests/SyncTextWriter.cs +++ b/src/libraries/System.Console/tests/SyncTextWriter.cs @@ -12,8 +12,8 @@ public class SyncTextWriter { - [Fact] - [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] + // Browser bypasses SyncTextWriter for faster startup + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void SyncTextWriterLockedOnThis() { TextWriter oldWriter = Console.Out; diff --git a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs index bf50637d5f7ac..29677f512d9f7 100644 --- a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs +++ b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.cs @@ -29,8 +29,8 @@ public void ObjectClosedReadLineBaseStream() Assert.Throws(() => sr.ReadLine()); } - [Fact] - [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] + // Browser bypasses SyncTextWriter for faster startup + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void Synchronized_NewObject() { using (Stream str = GetLargeStream()) diff --git a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs index eb4718aaf0f2e..cc09f1c0d2032 100644 --- a/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs +++ b/src/libraries/System.Runtime/tests/System.IO.Tests/StreamWriter/StreamWriter.cs @@ -7,8 +7,8 @@ namespace System.IO.Tests { public partial class WriteTests { - [Fact] - [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] + // Browser bypasses SyncTextWriter for faster startup + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void Synchronized_NewObject() { using (Stream str = CreateStream()) diff --git a/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs b/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs index d021010e54d9d..47541635c8eef 100644 --- a/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs +++ b/src/libraries/System.Runtime/tests/System.IO.Tests/TextWriter/TextWriterTests.cs @@ -690,8 +690,8 @@ public void DisposeAsync_ExceptionReturnedInTask() Assert.Same(e, vt.AsTask().Exception.InnerException); } - [Fact] - [SkipOnPlatform(TestPlatforms.Browser, "Browser bypasses SyncTextWriter for faster startup")] + // Browser bypasses SyncTextWriter for faster startup + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public async Task FlushAsync_Precanceled() { Assert.Equal(TaskStatus.RanToCompletion, TextWriter.Null.FlushAsync(new CancellationToken(true)).Status);