Skip to content

Commit

Permalink
Merge changes from a single DidChange notification
Browse files Browse the repository at this point in the history
I'm currently investigating a customer profile where didChange notifications are showing very prevalently in CPU time. I've create this PR (dotnet#74267) to help with the underlying performance of creating the SourceText, however, there would still be potentially many source texts created for a single operation.

This PR attempts to alleviate that situation by changing the DidChange handler to create a single source text on the input TextDocumentContentChangeEvent array. This array can be quite large, such as when the user does a replace-all or multi-line edit in VS.

Roughly 50% of CPU time in the customer profile was spent in our DidChange handler (about 29 seconds of CPU time).
  • Loading branch information
ToddGrun committed Jul 4, 2024
1 parent 967907d commit f489ce0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 5 deletions.
15 changes: 15 additions & 0 deletions src/LanguageServer/Protocol/Extensions/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,5 +243,20 @@ private static bool TryGetVSCompletionListSetting(ClientCapabilities clientCapab

return true;
}

public static int CompareTo(this Position p1, Position p2)
{
if (p1.Line > p2.Line)
return 1;
else if (p1.Line < p2.Line)
return -1;

if (p1.Character > p2.Character)
return 1;
else if (p1.Character < p2.Character)
return -1;

return 0;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

using System;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Text;
using Roslyn.LanguageServer.Protocol;
using Roslyn.Utilities;

Expand All @@ -28,15 +30,54 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(DidChangeTextDocumentPar
{
var text = context.GetTrackedDocumentSourceText(request.TextDocument.Uri);

text = UpdateSourceText(request.ContentChanges, text);

context.UpdateTrackedDocument(request.TextDocument.Uri, text);

return SpecializedTasks.Default<object>();
}

private static SourceText UpdateSourceText(TextDocumentContentChangeEvent[] contentChanges, SourceText text)
{
var areChangesOrdered = true;

// Per the LSP spec, each text change builds upon the previous, so we don't need to translate any text
// positions between changes, which makes this quite easy. See
// positions between changes. See
// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#didChangeTextDocumentParams
// for more details.
foreach (var change in request.ContentChanges)
text = text.WithChanges(ProtocolConversions.ContentChangeEventToTextChange(change, text));
if (contentChanges.Length > 0)
{
// If the host sends us changes in a way such that no earlier change can affect the position of a later change,
// then we can merge the changes into a single TextChange, allowing creation of only a single new
// source text.
for (var i = 1; i < contentChanges.Length; i++)
{
var prevChange = contentChanges[i - 1];
var curChange = contentChanges[i];

context.UpdateTrackedDocument(request.TextDocument.Uri, text);
if (prevChange.Range.Start.CompareTo(curChange.Range.End) < 0)
{
areChangesOrdered = false;
break;
}
}
}

return SpecializedTasks.Default<object>();
if (!areChangesOrdered)
{
// The host didn't send us the items ordered, so we'll apply each one independently.
foreach (var change in contentChanges)
text = text.WithChanges(ProtocolConversions.ContentChangeEventToTextChange(change, text));
}
else
{
// The changes were ordered, so we can merge them into a single operation on the source text. Note that
// the original change ordering was in reverse document order, whereas the WithChanges implementation works
// more efficiently with them in forward document order.
var newChanges = contentChanges.Reverse().SelectAsArray(change => ProtocolConversions.ContentChangeEventToTextChange(change, text));
text = text.WithChanges(newChanges);
}

return text;
}
}

0 comments on commit f489ce0

Please sign in to comment.