Merge changes from a single DidChange notification (#74268)

* Merge changes from a single DidChange notification

I'm currently investigating a customer profile where didChange notifications are showing very prevalently in CPU time. I've create this PR (#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).

Author: ToddGrun

src/LanguageServer/Protocol/Extensions/Extensions.cs

@@ -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;
+        }
     }
 }

src/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs

@@ -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;
 
@@ -28,15 +30,53 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(DidChangeTextDocumentPar
     {
         var text = context.GetTrackedDocumentSourceText(request.TextDocument.Uri);
 
-        // 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
-        // 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));
+        text = GetUpdatedSourceText(request.ContentChanges, text);
 
         context.UpdateTrackedDocument(request.TextDocument.Uri, text);
 
         return SpecializedTasks.Default<object>();
     }
+
+    internal static bool AreChangesInReverseOrder(TextDocumentContentChangeEvent[] contentChanges)
+    {
+        for (var i = 1; i < contentChanges.Length; i++)
+        {
+            var prevChange = contentChanges[i - 1];
+            var curChange = contentChanges[i];
+
+            if (prevChange.Range.Start.CompareTo(curChange.Range.End) < 0)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private static SourceText GetUpdatedSourceText(TextDocumentContentChangeEvent[] contentChanges, SourceText text)
+    {
+        // Per the LSP spec, each text change builds upon the previous, so we don't need to translate any text
+        // positions between changes. See
+        // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#didChangeTextDocumentParams
+        // for more details.
+        //
+        // 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.
+        if (AreChangesInReverseOrder(contentChanges))
+        {
+            // The changes were in reverse document order, so we can merge them into a single operation on the source text.
+            // Note that the WithChanges implementation works more efficiently with it's input in forward document order.
+            var newChanges = contentChanges.Reverse().SelectAsArray(change => ProtocolConversions.ContentChangeEventToTextChange(change, text));
+            text = text.WithChanges(newChanges);
+        }
+        else
+        {
+            // 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));
+        }
+
+        return text;
+    }
 }

src/LanguageServer/ProtocolUnitTests/DocumentChanges/DocumentChangesTests.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Linq;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.LanguageServer.Handler.DocumentChanges;
 using Roslyn.Test.Utilities;
 using Xunit;
 using Xunit.Abstractions;
@@ -245,7 +246,7 @@ void M()
         }
 
         [Theory, CombinatorialData]
-        public async Task DidChange_MultipleChanges1(bool mutatingLspWorkspace)
+        public async Task DidChange_MultipleChanges_ForwardOrder(bool mutatingLspWorkspace)
         {
             var source =
                 """
@@ -285,7 +286,7 @@ void M()
         }
 
         [Theory, CombinatorialData]
-        public async Task DidChange_MultipleChanges2(bool mutatingLspWorkspace)
+        public async Task DidChange_MultipleChanges_Overlapping(bool mutatingLspWorkspace)
         {
             var source =
                 """
@@ -323,6 +324,89 @@ void M()
             }
         }
 
+        [Theory, CombinatorialData]
+        public async Task DidChange_MultipleChanges_ReverseOrder(bool mutatingLspWorkspace)
+        {
+            var source =
+                """
+                class A
+                {
+                    void M()
+                    {
+                        {|type:|}
+                    }
+                }
+                """;
+            var expected =
+                """
+                class A
+                {
+                    void M()
+                    {
+                        // hi there
+                        // this builds on that
+                    }
+                }
+                """;
+
+            var (testLspServer, locationTyped, _) = await GetTestLspServerAndLocationAsync(source, mutatingLspWorkspace);
+
+            await using (testLspServer)
+            {
+                await DidOpen(testLspServer, locationTyped.Uri);
+
+                await DidChange(testLspServer, locationTyped.Uri, (5, 0, "        // this builds on that\r\n"), (4, 8, "// hi there"));
+
+                var document = testLspServer.GetTrackedTexts().FirstOrDefault();
+
+                AssertEx.NotNull(document);
+                Assert.Equal(expected, document.ToString());
+            }
+        }
+
+        private LSP.TextDocumentContentChangeEvent CreateTextDocumentContentChangeEvent(int startLine, int startCol, int endLine, int endCol, string newText)
+        {
+            return new LSP.TextDocumentContentChangeEvent()
+            {
+                Range = new LSP.Range()
+                {
+                    Start = new LSP.Position(startLine, startCol),
+                    End = new LSP.Position(endLine, endCol)
+                },
+                Text = newText
+            };
+        }
+
+        [Fact]
+        public void DidChange_AreChangesInReverseOrder_True()
+        {
+            LSP.TextDocumentContentChangeEvent change1 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 7, endLine: 0, endCol: 9, newText: "test3");
+            LSP.TextDocumentContentChangeEvent change2 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 5, endLine: 0, endCol: 7, newText: "test2");
+            LSP.TextDocumentContentChangeEvent change3 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 1, endLine: 0, endCol: 3, newText: "test1");
+
+            Assert.True(DidChangeHandler.AreChangesInReverseOrder([change1, change2, change3]));
+        }
+
+        [Fact]
+        public void DidChange_AreChangesInReverseOrder_InForwardOrder()
+        {
+            LSP.TextDocumentContentChangeEvent change1 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 1, endLine: 0, endCol: 3, newText: "test1");
+            LSP.TextDocumentContentChangeEvent change2 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 5, endLine: 0, endCol: 7, newText: "test2");
+            LSP.TextDocumentContentChangeEvent change3 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 7, endLine: 0, endCol: 9, newText: "test3");
+
+            Assert.False(DidChangeHandler.AreChangesInReverseOrder([change1, change2, change3]));
+        }
+
+        [Fact]
+        public void DidChange_AreChangesInReverseOrder_Overlapping()
+        {
+            LSP.TextDocumentContentChangeEvent change1 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 1, endLine: 0, endCol: 3, newText: "test1");
+            LSP.TextDocumentContentChangeEvent change2 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 2, endLine: 0, endCol: 4, newText: "test2");
+            LSP.TextDocumentContentChangeEvent change3 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 3, endLine: 0, endCol: 5, newText: "test3");
+
+            Assert.False(DidChangeHandler.AreChangesInReverseOrder([change1, change2, change3]));
+        }
+
         [Theory, CombinatorialData]
         public async Task DidChange_MultipleRequests(bool mutatingLspWorkspace)
         {