From 1f59612ad12e9d9b3c1987c932d9b7992d6dbdc0 Mon Sep 17 00:00:00 2001
From: whitema <whitema@deshaw.com>
Date: Tue, 31 Jul 2018 13:12:31 -0400
Subject: [PATCH] Reduce allocations when parsing files

The previous implementation used LINQ on a relatively hot path,
returned a lazy IEnumerable that was always eagerly converted to a
List or Array and allocated two strings per line when Windows
linebreaks were used.  Profiling indicates that PSES spends a huge
percentage of its time in GC, so switching to a slightly more complex
implementation that allocates less than half as much seems justified.
---
 .../Server/LanguageServer.cs                  |  9 ++++--
 .../Workspace/ScriptFile.cs                   | 25 ++++++++++++++--
 .../Session/ScriptFileTests.cs                | 29 +++++++++++++++++++
 3 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
index 684d86d7c..cfdebc527 100644
--- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
+++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
@@ -1096,6 +1096,7 @@ protected async Task HandleCommentHelpRequest(
                 triggerLine1b,
                 out helpLocation);
             var result = new CommentHelpRequestResult();
+            IList<string> lines = null;
             if (functionDefinitionAst != null)
             {
                 var funcExtent = functionDefinitionAst.Extent;
@@ -1104,7 +1105,7 @@ protected async Task HandleCommentHelpRequest(
                 {
                     // check if the previous character is `<` because it invalidates
                     // the param block the follows it.
-                    var lines = ScriptFile.GetLines(funcText).ToArray();
+                    lines = ScriptFile.GetLines(funcText);
                     var relativeTriggerLine0b = triggerLine1b - funcExtent.StartLineNumber;
                     if (relativeTriggerLine0b > 0 && lines[relativeTriggerLine0b].IndexOf("<") > -1)
                     {
@@ -1122,8 +1123,12 @@ protected async Task HandleCommentHelpRequest(
                         requestParams.BlockComment,
                         true,
                         helpLocation));
+
                 var help = analysisResults?.FirstOrDefault()?.Correction?.Edits[0].Text;
-                result.Content = help == null ? null : ScriptFile.GetLines(help).ToArray();
+                result.Content = help != null 
+                    ? (lines ?? ScriptFile.GetLines(funcText)).ToArray() 
+                    : null;
+
                 if (helpLocation != null &&
                     !helpLocation.Equals("before", StringComparison.OrdinalIgnoreCase))
                 {
diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs
index fc290cff0..9d3b4aead 100644
--- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs
+++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs
@@ -197,14 +197,33 @@ public ScriptFile(
         /// </summary>
         /// <param name="text">Input string to be split up into lines.</param>
         /// <returns>The lines in the string.</returns>
-        public static IEnumerable<string> GetLines(string text)
+        public static IList<string> GetLines(string text)
         {
             if (text == null)
             {
                 throw new ArgumentNullException(nameof(text));
             }
 
-            return text.Split('\n').Select(line => line.TrimEnd('\r'));
+            // ReadLine returns null immediately for empty string, so special case it.
+            if (text.Length == 0)
+            {
+                return new List<string> {string.Empty};
+            }
+
+            using (var reader = new StringReader(text))
+            {
+                // 50 is a rough guess for typical average line length, this saves some list
+                // resizes in the common case and does not hurt meaningfully if we're wrong.
+                var list = new List<string>(text.Length / 50);
+                string line;
+
+                while ((line = reader.ReadLine()) != null)
+                {
+                    list.Add(line);
+                }
+
+                return list;
+            }
         }
 
         /// <summary>
@@ -517,7 +536,7 @@ private void SetFileContents(string fileContents)
         {
             // Split the file contents into lines and trim
             // any carriage returns from the strings.
-            this.FileLines = GetLines(fileContents).ToList();
+            this.FileLines = GetLines(fileContents);
 
             // Parse the contents to get syntax tree and errors
             this.ParseFileContents();
diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs
index 2045f6fd5..ccaed4ce3 100644
--- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs
+++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs
@@ -296,6 +296,35 @@ public void CanGetRangeAtLineBoundaries()
 
             Assert.Equal(expectedLines, lines);
         }
+
+        [Fact]
+        public void CanSplitLines()
+        {
+            Assert.Equal(TestStringLines, scriptFile.FileLines);
+        }
+
+        [Fact]
+        public void CanGetSameLinesWithUnixLineBreaks()
+        {
+            var unixFile = ScriptFileChangeTests.CreateScriptFile(TestString.Replace("\r\n", "\n"));
+            Assert.Equal(scriptFile.FileLines, unixFile.FileLines);
+        }
+
+        [Fact]
+        public void CanGetLineForEmptyString()
+        {
+            var emptyFile = ScriptFileChangeTests.CreateScriptFile(string.Empty);
+            Assert.Equal(1, emptyFile.FileLines.Count);
+            Assert.Equal(string.Empty, emptyFile.FileLines[0]);
+        }
+
+        [Fact]
+        public void CanGetLineForSpace()
+        {
+            var spaceFile = ScriptFileChangeTests.CreateScriptFile(" ");
+            Assert.Equal(1, spaceFile.FileLines.Count);
+            Assert.Equal(" ", spaceFile.FileLines[0]);
+        }
     }
 
     public class ScriptFilePositionTests