Replace ReusableTextReader with shared ArrayPool (#43365)
authorAdeel Mujahid <3840695+am11@users.noreply.github.com>
Wed, 14 Oct 2020 04:24:02 +0000 (07:24 +0300)
committerGitHub <noreply@github.com>
Wed, 14 Oct 2020 04:24:02 +0000 (21:24 -0700)
src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs
src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs
src/libraries/Common/src/System/Text/ReusableTextReader.cs [deleted file]
src/libraries/Common/tests/Common.Tests.csproj
src/libraries/Common/tests/Tests/Interop/procfsTests.cs
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessThread.Linux.cs
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
src/libraries/System.Private.CoreLib/src/System/Environment.Linux.cs

index 0187e77..503f972 100644 (file)
@@ -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<byte>.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<byte>.Shared.Rent(bytes.Length * 2);
+                        Array.Copy(bytes, temp, bytes.Length);
+                        byte[] toReturn = bytes;
+                        bytes = temp;
+                        ArrayPool<byte>.Shared.Return(toReturn);
+                    }
                 }
             }
             catch (IOException)
             {
-                fileContents = null;
+                contents = null;
                 return false;
             }
+            finally
+            {
+                ArrayPool<byte>.Shared.Return(bytes);
+            }
         }
     }
 }
index 42a6b5c..45d484e 100644 (file)
@@ -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 (file)
index 9fb30f3..0000000
+++ /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
-{
-    /// <summary>Provides a reusable reader for reading all of the text from streams.</summary>
-    internal sealed class ReusableTextReader
-    {
-        /// <summary>StringBuilder used to store intermediate text results.</summary>
-        private readonly StringBuilder _builder = new StringBuilder();
-        /// <summary>Decoder used to decode data read from the stream.</summary>
-        private readonly Decoder _decoder;
-        /// <summary>Bytes read from the stream.</summary>
-        private readonly byte[] _bytes;
-        /// <summary>Temporary storage from characters converted from the bytes then written to the builder.</summary>
-        private readonly char[] _chars;
-
-        /// <summary>Initializes a new reusable reader.</summary>
-        /// <param name="encoding">The Encoding to use.  Defaults to UTF8.</param>
-        /// <param name="bufferSize">The size of the buffer to use when reading from the stream.</param>
-        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)];
-        }
-
-        /// <summary>Read all of the text from the current position of the stream.</summary>
-        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;
-        }
-    }
-}
index 698a49c..b7c9f6b 100644 (file)
@@ -66,8 +66,6 @@
              Link="Common\System\Net\Http\aspnetcore\Http2\Hpack\H2StaticTable.cs" />
     <Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http2\Hpack\StatusCodes.cs"
              Link="Common\System\Net\Http\aspnetcore\Http2\Hpack\StatusCodes.cs" />
-    <Compile Include="$(CommonPath)System\Text\ReusableTextReader.cs"
-             Link="Common\System\Text\ReusableTextReader.cs" />
     <Compile Include="$(CommonPath)System\Text\SimpleRegex.cs"
              Link="Common\System\Text\SimpleRegex.cs" />
     <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
index 464569a..c585be7 100644 (file)
@@ -45,7 +45,7 @@ namespace Common.Tests
                 File.WriteAllText(path, statFileText);
 
                 Interop.procfs.ParsedStat result;
-                Assert.True(Interop.procfs.TryParseStatFile(path, out result, new ReusableTextReader()));
+                Assert.True(Interop.procfs.TryParseStatFile(path, out result));
 
                 Assert.Equal(expectedPid, result.pid);
                 Assert.Equal(expectedComm, result.comm);
index fcfeef1..e9f2630 100644 (file)
              Link="Common\Interop\Linux\Interop.ProcFsStat.TryReadStatusFile.cs" />
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.SchedGetSetAffinity.cs"
              Link="Common\Interop\Linux\Interop.SchedGetSetAffinity.cs" />
-    <Compile Include="$(CommonPath)System\Text\ReusableTextReader.cs"
-             Link="Common\System\Text\ReusableTextReader.cs" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsOSX)' == 'true' or '$(TargetsiOS)' == 'true' or '$(TargetstvOS)' == 'true'">
     <Compile Include="System\Diagnostics\Process.BSD.cs" />
index 68019f6..775c170 100644 (file)
@@ -25,15 +25,14 @@ namespace System.Diagnostics
                 processName = string.Empty;
             }
 
-            var reusableReader = new ReusableTextReader();
             var processes = new List<Process>();
             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);
             }
index e78d807..cc51476 100644 (file)
@@ -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<ProcessInfo>(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
         /// <summary>
         /// Creates a ProcessInfo from the specified process ID.
         /// </summary>
-        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
         /// <summary>
         /// Creates a ProcessInfo from the data parsed from a /proc/pid/stat file and the associated tasks directory.
         /// </summary>
-        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
                         {
index b57a687..3a0f234 100644 (file)
@@ -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));
             }
index 28ab1fe..83c1ebe 100644 (file)
              Link="Common\Interop\BSD\System.Native\Interop.Sysctl.cs" />
     <Compile Include="$(CommonPath)Interop\Linux\procfs\Interop.ProcFsStat.TryReadStatusFile.cs" Condition="'$(TargetsLinux)' == 'true'"
              Link="Common\Interop\Linux\Interop.ProcFsStat.TryReadStatusFile.cs" />
-    <Compile Include="$(CommonPath)System\Text\ReusableTextReader.cs" Condition="'$(TargetsLinux)' == 'true'"
-             Link="Common\System\Text\ReusableTextReader.cs" />
     <Compile Include="$(CommonPath)System\IO\StringParser.cs" Condition="'$(TargetsLinux)' == 'true'"
              Link="Common\System\IO\StringParser.cs" />
     <Compile Include="$(CommonPath)Interop\OSX\Interop.libproc.GetProcessInfoById.cs" Condition="'$(IsOSXLike)' == 'true'"
index ea6eafe..4f488f4 100644 (file)
@@ -2,13 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Runtime.InteropServices;
-using System.Text;
 
 namespace System
 {
     public static partial class Environment
     {
-        private static ReusableTextReader reusableReader = new ReusableTextReader();
-        public static long WorkingSet => (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);
     }
 }