From a52ea3cabe34c6e1c90a6126ab5a506151ca6362 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 6 Jul 2022 14:29:55 +0900 Subject: [PATCH 1/4] Enable NRT and simplify `LineBufferedReader` --- .../Beatmaps/IO/LineBufferedReaderTest.cs | 12 ++--- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 2 +- osu.Game/IO/LineBufferedReader.cs | 48 ++++++++----------- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs b/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs index 8f20fd7a68..14fbcb6176 100644 --- a/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs @@ -108,19 +108,13 @@ namespace osu.Game.Tests.Beatmaps.IO [Test] public void TestReadToEndAfterReadsAndPeeks() { - const string contents = "this line is gone\rthis one shouldn't be\r\nthese ones\ndefinitely not"; + const string contents = "first line\r\nsecond line"; using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) using (var bufferedReader = new LineBufferedReader(stream)) { - Assert.AreEqual("this line is gone", bufferedReader.ReadLine()); - Assert.AreEqual("this one shouldn't be", bufferedReader.PeekLine()); - - string[] endingLines = bufferedReader.ReadToEnd().Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); - Assert.AreEqual(3, endingLines.Length); - Assert.AreEqual("this one shouldn't be", endingLines[0]); - Assert.AreEqual("these ones", endingLines[1]); - Assert.AreEqual("definitely not", endingLines[2]); + bufferedReader.PeekLine(); + Assert.Throws(() => bufferedReader.ReadToEnd()); } } } diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index a5e6ac0a1c..52e760a068 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -29,7 +29,7 @@ namespace osu.Game.Beatmaps.Formats { Section section = Section.General; - string line; + string? line; while ((line = stream.ReadLine()) != null) { diff --git a/osu.Game/IO/LineBufferedReader.cs b/osu.Game/IO/LineBufferedReader.cs index db435576bf..1e329c786f 100644 --- a/osu.Game/IO/LineBufferedReader.cs +++ b/osu.Game/IO/LineBufferedReader.cs @@ -1,10 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; -using System.Collections.Generic; using System.IO; using System.Text; @@ -17,58 +14,51 @@ namespace osu.Game.IO public class LineBufferedReader : IDisposable { private readonly StreamReader streamReader; - private readonly Queue lineBuffer; + + private string? peekedLine; public LineBufferedReader(Stream stream, bool leaveOpen = false) { streamReader = new StreamReader(stream, Encoding.UTF8, true, 1024, leaveOpen); - lineBuffer = new Queue(); } /// /// Reads the next line from the stream without consuming it. /// Subsequent calls to without a will return the same string. /// - public string PeekLine() - { - if (lineBuffer.Count > 0) - return lineBuffer.Peek(); - - string line = streamReader.ReadLine(); - if (line != null) - lineBuffer.Enqueue(line); - return line; - } + public string? PeekLine() => peekedLine ??= streamReader.ReadLine(); /// /// Reads the next line from the stream and consumes it. /// If a line was peeked, that same line will then be consumed and returned. /// - public string ReadLine() => lineBuffer.Count > 0 ? lineBuffer.Dequeue() : streamReader.ReadLine(); + public string? ReadLine() + { + try + { + return peekedLine ?? streamReader.ReadLine(); + } + finally + { + peekedLine = null; + } + } /// /// Reads the stream to its end and returns the text read. - /// This includes any peeked but unconsumed lines. + /// Not compatible with calls to . /// public string ReadToEnd() { - string remainingText = streamReader.ReadToEnd(); - if (lineBuffer.Count == 0) - return remainingText; + if (peekedLine != null) + throw new InvalidOperationException($"Do not use {nameof(ReadToEnd)} when also peeking for lines."); - var builder = new StringBuilder(); - - // this might not be completely correct due to varying platform line endings - while (lineBuffer.Count > 0) - builder.AppendLine(lineBuffer.Dequeue()); - builder.Append(remainingText); - - return builder.ToString(); + return streamReader.ReadToEnd(); } public void Dispose() { - streamReader?.Dispose(); + streamReader.Dispose(); } } } From 12d396a51359dcec41f24a34aa8424f43d5acc2f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 6 Jul 2022 15:43:35 +0900 Subject: [PATCH 2/4] Use `-1` to specify default buffer size --- osu.Game/IO/LineBufferedReader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/IO/LineBufferedReader.cs b/osu.Game/IO/LineBufferedReader.cs index 1e329c786f..0210d7207e 100644 --- a/osu.Game/IO/LineBufferedReader.cs +++ b/osu.Game/IO/LineBufferedReader.cs @@ -19,7 +19,7 @@ namespace osu.Game.IO public LineBufferedReader(Stream stream, bool leaveOpen = false) { - streamReader = new StreamReader(stream, Encoding.UTF8, true, 1024, leaveOpen); + streamReader = new StreamReader(stream, Encoding.UTF8, true, -1, leaveOpen); } /// From 01bc6e5cb7ef98d26e001b8f89bef14fd1ddabd7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 6 Jul 2022 19:53:19 +0900 Subject: [PATCH 3/4] Revert old behaviour of `ReadToEnd` --- .../Beatmaps/IO/LineBufferedReaderTest.cs | 12 +++++++++--- osu.Game/IO/LineBufferedReader.cs | 15 +++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs b/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs index 14fbcb6176..8f20fd7a68 100644 --- a/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs @@ -108,13 +108,19 @@ namespace osu.Game.Tests.Beatmaps.IO [Test] public void TestReadToEndAfterReadsAndPeeks() { - const string contents = "first line\r\nsecond line"; + const string contents = "this line is gone\rthis one shouldn't be\r\nthese ones\ndefinitely not"; using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) using (var bufferedReader = new LineBufferedReader(stream)) { - bufferedReader.PeekLine(); - Assert.Throws(() => bufferedReader.ReadToEnd()); + Assert.AreEqual("this line is gone", bufferedReader.ReadLine()); + Assert.AreEqual("this one shouldn't be", bufferedReader.PeekLine()); + + string[] endingLines = bufferedReader.ReadToEnd().Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + Assert.AreEqual(3, endingLines.Length); + Assert.AreEqual("this one shouldn't be", endingLines[0]); + Assert.AreEqual("these ones", endingLines[1]); + Assert.AreEqual("definitely not", endingLines[2]); } } } diff --git a/osu.Game/IO/LineBufferedReader.cs b/osu.Game/IO/LineBufferedReader.cs index 0210d7207e..6fcea08990 100644 --- a/osu.Game/IO/LineBufferedReader.cs +++ b/osu.Game/IO/LineBufferedReader.cs @@ -46,14 +46,21 @@ namespace osu.Game.IO /// /// Reads the stream to its end and returns the text read. - /// Not compatible with calls to . + /// This includes any peeked but unconsumed lines. /// public string ReadToEnd() { - if (peekedLine != null) - throw new InvalidOperationException($"Do not use {nameof(ReadToEnd)} when also peeking for lines."); + string remainingText = streamReader.ReadToEnd(); + if (peekedLine == null) + return remainingText; - return streamReader.ReadToEnd(); + var builder = new StringBuilder(); + + // this might not be completely correct due to varying platform line endings + builder.AppendLine(peekedLine); + builder.Append(remainingText); + + return builder.ToString(); } public void Dispose() From c2f106907364de79167a92cf8e0f2f1930168449 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 6 Jul 2022 19:55:43 +0900 Subject: [PATCH 4/4] Avoid usage of `finally` in potentially hot path --- osu.Game/IO/LineBufferedReader.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/osu.Game/IO/LineBufferedReader.cs b/osu.Game/IO/LineBufferedReader.cs index 6fcea08990..93e554b43d 100644 --- a/osu.Game/IO/LineBufferedReader.cs +++ b/osu.Game/IO/LineBufferedReader.cs @@ -34,14 +34,10 @@ namespace osu.Game.IO /// public string? ReadLine() { - try - { - return peekedLine ?? streamReader.ReadLine(); - } - finally - { - peekedLine = null; - } + string? line = peekedLine ?? streamReader.ReadLine(); + + peekedLine = null; + return line; } ///