From 041826bf439bf72a4bb69f0eed74a40d4955b544 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Wed, 14 Oct 2020 07:24:02 +0300 Subject: [PATCH] Replace ReusableTextReader with shared ArrayPool (#43365) --- .../procfs/Interop.ProcFsStat.TryReadStatusFile.cs | 44 +++++++++++++---- .../src/Interop/Linux/procfs/Interop.ProcFsStat.cs | 12 ++--- .../Common/src/System/Text/ReusableTextReader.cs | 55 ---------------------- src/libraries/Common/tests/Common.Tests.csproj | 2 - .../Common/tests/Tests/Interop/procfsTests.cs | 2 +- .../src/System.Diagnostics.Process.csproj | 2 - .../src/System/Diagnostics/Process.Linux.cs | 9 ++-- .../src/System/Diagnostics/ProcessManager.Linux.cs | 16 +++---- .../src/System/Diagnostics/ProcessThread.Linux.cs | 2 +- .../src/System.Private.CoreLib.Shared.projitems | 2 - .../src/System/Environment.Linux.cs | 4 +- 11 files changed, 55 insertions(+), 95 deletions(-) delete mode 100644 src/libraries/Common/src/System/Text/ReusableTextReader.cs diff --git a/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs b/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs index 0187e77..503f972 100644 --- a/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs +++ b/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs @@ -3,6 +3,7 @@ #nullable enable using System; +using System.Buffers; using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -35,18 +36,18 @@ internal static partial class Interop return RootPath + pid.ToString(CultureInfo.InvariantCulture) + StatusFileName; } - internal static bool TryReadStatusFile(int pid, out ParsedStatus result, ReusableTextReader reusableReader) + internal static bool TryReadStatusFile(int pid, out ParsedStatus result) { - bool b = TryParseStatusFile(GetStatusFilePathForProcess(pid), out result, reusableReader); + bool b = TryParseStatusFile(GetStatusFilePathForProcess(pid), out result); #if DEBUG Debug.Assert(!b || result.Pid == pid, "Expected process ID from status file to match supplied pid"); #endif return b; } - internal static bool TryParseStatusFile(string statusFilePath, out ParsedStatus result, ReusableTextReader reusableReader) + internal static bool TryParseStatusFile(string statusFilePath, out ParsedStatus result) { - if (!TryReadFile(statusFilePath, reusableReader, out string? fileContents)) + if (!TryReadFile(statusFilePath, out string? fileContents)) { // Between the time that we get an ID and the time that we try to read the associated stat // file(s), the process could be gone. @@ -149,21 +150,46 @@ internal static partial class Interop return true; } - private static bool TryReadFile(string filePath, ReusableTextReader reusableReader, [NotNullWhen(true)] out string? fileContents) + private static bool TryReadFile(string path, [NotNullWhen(true)] out string? contents) { + Debug.Assert(!string.IsNullOrEmpty(path)); + + byte[] bytes = ArrayPool.Shared.Rent(4096); + int count = 0; + try { - using (var fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1, useAsync: false)) + using var fileStream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1, useAsync: false); + + while (true) { - fileContents = reusableReader.ReadAllText(fileStream); - return true; + int read = fileStream.Read(bytes, count, bytes.Length - count); + if (read == 0) + { + contents = Encoding.UTF8.GetString(bytes, 0, count); + return true; + } + + count += read; + if (count >= bytes.Length) + { + byte[] temp = ArrayPool.Shared.Rent(bytes.Length * 2); + Array.Copy(bytes, temp, bytes.Length); + byte[] toReturn = bytes; + bytes = temp; + ArrayPool.Shared.Return(toReturn); + } } } catch (IOException) { - fileContents = null; + contents = null; return false; } + finally + { + ArrayPool.Shared.Return(bytes); + } } } } diff --git a/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs b/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs index 42a6b5c..45d484e 100644 --- a/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs +++ b/src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs @@ -199,23 +199,23 @@ internal static partial class Interop return GetTaskDirectoryPathForProcess(pid) + tid.ToString(CultureInfo.InvariantCulture) + StatFileName; } - internal static bool TryReadStatFile(int pid, out ParsedStat result, ReusableTextReader reusableReader) + internal static bool TryReadStatFile(int pid, out ParsedStat result) { - bool b = TryParseStatFile(GetStatFilePathForProcess(pid), out result, reusableReader); + bool b = TryParseStatFile(GetStatFilePathForProcess(pid), out result); Debug.Assert(!b || result.pid == pid, "Expected process ID from stat file to match supplied pid"); return b; } - internal static bool TryReadStatFile(int pid, int tid, out ParsedStat result, ReusableTextReader reusableReader) + internal static bool TryReadStatFile(int pid, int tid, out ParsedStat result) { - bool b = TryParseStatFile(GetStatFilePathForThread(pid, tid), out result, reusableReader); + bool b = TryParseStatFile(GetStatFilePathForThread(pid, tid), out result); Debug.Assert(!b || result.pid == tid, "Expected thread ID from stat file to match supplied tid"); return b; } - internal static bool TryParseStatFile(string statFilePath, out ParsedStat result, ReusableTextReader reusableReader) + internal static bool TryParseStatFile(string statFilePath, out ParsedStat result) { - if (!TryReadFile(statFilePath, reusableReader, out string? statFileContents)) + if (!TryReadFile(statFilePath, out string? statFileContents)) { // Between the time that we get an ID and the time that we try to read the associated stat // file(s), the process could be gone. diff --git a/src/libraries/Common/src/System/Text/ReusableTextReader.cs b/src/libraries/Common/src/System/Text/ReusableTextReader.cs deleted file mode 100644 index 9fb30f3..0000000 --- a/src/libraries/Common/src/System/Text/ReusableTextReader.cs +++ /dev/null @@ -1,55 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.IO; - -#nullable enable - -namespace System.Text -{ - /// Provides a reusable reader for reading all of the text from streams. - internal sealed class ReusableTextReader - { - /// StringBuilder used to store intermediate text results. - private readonly StringBuilder _builder = new StringBuilder(); - /// Decoder used to decode data read from the stream. - private readonly Decoder _decoder; - /// Bytes read from the stream. - private readonly byte[] _bytes; - /// Temporary storage from characters converted from the bytes then written to the builder. - private readonly char[] _chars; - - /// Initializes a new reusable reader. - /// The Encoding to use. Defaults to UTF8. - /// The size of the buffer to use when reading from the stream. - public ReusableTextReader(Encoding? encoding = null, int bufferSize = 1024) - { - if (encoding == null) - { - encoding = Encoding.UTF8; - } - - _decoder = encoding.GetDecoder(); - _bytes = new byte[bufferSize]; - _chars = new char[encoding.GetMaxCharCount(_bytes.Length)]; - } - - /// Read all of the text from the current position of the stream. - public unsafe string ReadAllText(Stream source) - { - int bytesRead; - while ((bytesRead = source.Read(_bytes, 0, _bytes.Length)) != 0) - { - int charCount = _decoder.GetChars(_bytes, 0, bytesRead, _chars, 0); - _builder.Append(_chars, 0, charCount); - } - - string s = _builder.ToString(); - - _builder.Clear(); - _decoder.Reset(); - - return s; - } - } -} diff --git a/src/libraries/Common/tests/Common.Tests.csproj b/src/libraries/Common/tests/Common.Tests.csproj index 698a49c..b7c9f6b 100644 --- a/src/libraries/Common/tests/Common.Tests.csproj +++ b/src/libraries/Common/tests/Common.Tests.csproj @@ -66,8 +66,6 @@ Link="Common\System\Net\Http\aspnetcore\Http2\Hpack\H2StaticTable.cs" /> - - diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs index 68019f6..775c170 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs @@ -25,15 +25,14 @@ namespace System.Diagnostics processName = string.Empty; } - var reusableReader = new ReusableTextReader(); var processes = new List(); foreach (int pid in ProcessManager.EnumerateProcessIds()) { - if (Interop.procfs.TryReadStatFile(pid, out Interop.procfs.ParsedStat parsedStat, reusableReader) && + if (Interop.procfs.TryReadStatFile(pid, out Interop.procfs.ParsedStat parsedStat) && string.Equals(processName, Process.GetUntruncatedProcessName(ref parsedStat), StringComparison.OrdinalIgnoreCase) && - Interop.procfs.TryReadStatusFile(pid, out Interop.procfs.ParsedStatus parsedStatus, reusableReader)) + Interop.procfs.TryReadStatusFile(pid, out Interop.procfs.ParsedStatus parsedStatus)) { - ProcessInfo processInfo = ProcessManager.CreateProcessInfo(ref parsedStat, ref parsedStatus, reusableReader, processName); + ProcessInfo processInfo = ProcessManager.CreateProcessInfo(ref parsedStat, ref parsedStatus, processName); processes.Add(new Process(machineName, false, processInfo.ProcessId, processInfo)); } } @@ -357,7 +356,7 @@ namespace System.Diagnostics { EnsureState(State.HaveNonExitedId); Interop.procfs.ParsedStat stat; - if (!Interop.procfs.TryReadStatFile(_processId, out stat, new ReusableTextReader())) + if (!Interop.procfs.TryReadStatFile(_processId, out stat)) { throw new Win32Exception(SR.ProcessInformationUnavailable); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs index e78d807..cc51476 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs @@ -26,11 +26,10 @@ namespace System.Diagnostics int[] procIds = GetProcessIds(machineName); // Iterate through all process IDs to load information about each process - var reusableReader = new ReusableTextReader(); var processes = new List(procIds.Length); foreach (int pid in procIds) { - ProcessInfo? pi = CreateProcessInfo(pid, reusableReader); + ProcessInfo? pi = CreateProcessInfo(pid); if (pi != null) { processes.Add(pi); @@ -103,13 +102,12 @@ namespace System.Diagnostics /// /// Creates a ProcessInfo from the specified process ID. /// - internal static ProcessInfo? CreateProcessInfo(int pid, ReusableTextReader? reusableReader = null) + internal static ProcessInfo? CreateProcessInfo(int pid) { - reusableReader ??= new ReusableTextReader(); - if (Interop.procfs.TryReadStatFile(pid, out Interop.procfs.ParsedStat stat, reusableReader)) + if (Interop.procfs.TryReadStatFile(pid, out Interop.procfs.ParsedStat stat)) { - Interop.procfs.TryReadStatusFile(pid, out Interop.procfs.ParsedStatus status, reusableReader); - return CreateProcessInfo(ref stat, ref status, reusableReader); + Interop.procfs.TryReadStatusFile(pid, out Interop.procfs.ParsedStatus status); + return CreateProcessInfo(ref stat, ref status); } return null; } @@ -117,7 +115,7 @@ namespace System.Diagnostics /// /// Creates a ProcessInfo from the data parsed from a /proc/pid/stat file and the associated tasks directory. /// - internal static ProcessInfo CreateProcessInfo(ref Interop.procfs.ParsedStat procFsStat, ref Interop.procfs.ParsedStatus procFsStatus, ReusableTextReader reusableReader, string? processName = null) + internal static ProcessInfo CreateProcessInfo(ref Interop.procfs.ParsedStat procFsStat, ref Interop.procfs.ParsedStatus procFsStatus, string? processName = null) { int pid = procFsStat.pid; @@ -151,7 +149,7 @@ namespace System.Diagnostics int tid; Interop.procfs.ParsedStat stat; if (int.TryParse(dirName, NumberStyles.Integer, CultureInfo.InvariantCulture, out tid) && - Interop.procfs.TryReadStatFile(pid, tid, out stat, reusableReader)) + Interop.procfs.TryReadStatFile(pid, tid, out stat)) { unsafe { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessThread.Linux.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessThread.Linux.cs index b57a687..3a0f234 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessThread.Linux.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessThread.Linux.cs @@ -77,7 +77,7 @@ namespace System.Diagnostics private Interop.procfs.ParsedStat GetStat() { Interop.procfs.ParsedStat stat; - if (!Interop.procfs.TryReadStatFile(pid: _processId, tid: Id, result: out stat, reusableReader: new ReusableTextReader(Encoding.UTF8))) + if (!Interop.procfs.TryReadStatFile(pid: _processId, tid: Id, result: out stat)) { throw new InvalidOperationException(SR.Format(SR.ThreadExited, Id)); } 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 28ab1fe..83c1ebe 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 @@ -1853,8 +1853,6 @@ Link="Common\Interop\BSD\System.Native\Interop.Sysctl.cs" /> - (long)(Interop.procfs.TryReadStatusFile(ProcessId, out Interop.procfs.ParsedStatus status, reusableReader) ? status.VmRSS : 0); + public static long WorkingSet => (long)(Interop.procfs.TryReadStatusFile(ProcessId, out Interop.procfs.ParsedStatus status) ? status.VmRSS : 0); } } -- 2.7.4