Skip to content

Commit 1f59612

Browse files
author
whitema
committed
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.
1 parent bd400a8 commit 1f59612

File tree

3 files changed

+58
-5
lines changed

3 files changed

+58
-5
lines changed

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,7 @@ protected async Task HandleCommentHelpRequest(
10961096
triggerLine1b,
10971097
out helpLocation);
10981098
var result = new CommentHelpRequestResult();
1099+
IList<string> lines = null;
10991100
if (functionDefinitionAst != null)
11001101
{
11011102
var funcExtent = functionDefinitionAst.Extent;
@@ -1104,7 +1105,7 @@ protected async Task HandleCommentHelpRequest(
11041105
{
11051106
// check if the previous character is `<` because it invalidates
11061107
// the param block the follows it.
1107-
var lines = ScriptFile.GetLines(funcText).ToArray();
1108+
lines = ScriptFile.GetLines(funcText);
11081109
var relativeTriggerLine0b = triggerLine1b - funcExtent.StartLineNumber;
11091110
if (relativeTriggerLine0b > 0 && lines[relativeTriggerLine0b].IndexOf("<") > -1)
11101111
{
@@ -1122,8 +1123,12 @@ protected async Task HandleCommentHelpRequest(
11221123
requestParams.BlockComment,
11231124
true,
11241125
helpLocation));
1126+
11251127
var help = analysisResults?.FirstOrDefault()?.Correction?.Edits[0].Text;
1126-
result.Content = help == null ? null : ScriptFile.GetLines(help).ToArray();
1128+
result.Content = help != null
1129+
? (lines ?? ScriptFile.GetLines(funcText)).ToArray()
1130+
: null;
1131+
11271132
if (helpLocation != null &&
11281133
!helpLocation.Equals("before", StringComparison.OrdinalIgnoreCase))
11291134
{

src/PowerShellEditorServices/Workspace/ScriptFile.cs

+22-3
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,33 @@ public ScriptFile(
197197
/// </summary>
198198
/// <param name="text">Input string to be split up into lines.</param>
199199
/// <returns>The lines in the string.</returns>
200-
public static IEnumerable<string> GetLines(string text)
200+
public static IList<string> GetLines(string text)
201201
{
202202
if (text == null)
203203
{
204204
throw new ArgumentNullException(nameof(text));
205205
}
206206

207-
return text.Split('\n').Select(line => line.TrimEnd('\r'));
207+
// ReadLine returns null immediately for empty string, so special case it.
208+
if (text.Length == 0)
209+
{
210+
return new List<string> {string.Empty};
211+
}
212+
213+
using (var reader = new StringReader(text))
214+
{
215+
// 50 is a rough guess for typical average line length, this saves some list
216+
// resizes in the common case and does not hurt meaningfully if we're wrong.
217+
var list = new List<string>(text.Length / 50);
218+
string line;
219+
220+
while ((line = reader.ReadLine()) != null)
221+
{
222+
list.Add(line);
223+
}
224+
225+
return list;
226+
}
208227
}
209228

210229
/// <summary>
@@ -517,7 +536,7 @@ private void SetFileContents(string fileContents)
517536
{
518537
// Split the file contents into lines and trim
519538
// any carriage returns from the strings.
520-
this.FileLines = GetLines(fileContents).ToList();
539+
this.FileLines = GetLines(fileContents);
521540

522541
// Parse the contents to get syntax tree and errors
523542
this.ParseFileContents();

test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs

+29
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,35 @@ public void CanGetRangeAtLineBoundaries()
296296

297297
Assert.Equal(expectedLines, lines);
298298
}
299+
300+
[Fact]
301+
public void CanSplitLines()
302+
{
303+
Assert.Equal(TestStringLines, scriptFile.FileLines);
304+
}
305+
306+
[Fact]
307+
public void CanGetSameLinesWithUnixLineBreaks()
308+
{
309+
var unixFile = ScriptFileChangeTests.CreateScriptFile(TestString.Replace("\r\n", "\n"));
310+
Assert.Equal(scriptFile.FileLines, unixFile.FileLines);
311+
}
312+
313+
[Fact]
314+
public void CanGetLineForEmptyString()
315+
{
316+
var emptyFile = ScriptFileChangeTests.CreateScriptFile(string.Empty);
317+
Assert.Equal(1, emptyFile.FileLines.Count);
318+
Assert.Equal(string.Empty, emptyFile.FileLines[0]);
319+
}
320+
321+
[Fact]
322+
public void CanGetLineForSpace()
323+
{
324+
var spaceFile = ScriptFileChangeTests.CreateScriptFile(" ");
325+
Assert.Equal(1, spaceFile.FileLines.Count);
326+
Assert.Equal(" ", spaceFile.FileLines[0]);
327+
}
299328
}
300329

301330
public class ScriptFilePositionTests

0 commit comments

Comments
 (0)