Reduce allocations in the CodeLens providers (#719)

rjmholt · web-flow · commit 06d3c223447b · 2018-08-24T13:28:27.000-07:00
* Stop creating new JsonSerializers

* Reduce allocating operations in CodeLens providers

* Fix style mistake

* Fix style mistake

* Remove unused class

* Use types instead of var

* Use SelectMany in simple non-linq case

* Fix empty array handling
diff --git a/src/PowerShellEditorServices.Host/CodeLens/CodeLensFeature.cs b/src/PowerShellEditorServices.Host/CodeLens/CodeLensFeature.cs
@@ -9,6 +9,7 @@
 using Microsoft.PowerShell.EditorServices.Utility;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
+using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -17,43 +18,43 @@
 
 namespace Microsoft.PowerShell.EditorServices.CodeLenses
 {
+    /// <summary>
+    /// Implements the CodeLens feature for EditorServices.
+    /// </summary>
     internal class CodeLensFeature :
         FeatureComponentBase<ICodeLensProvider>,
         ICodeLenses
     {
-        private EditorSession editorSession;
-
-        private JsonSerializer jsonSerializer =
-             JsonSerializer.Create(
-                 Constants.JsonSerializerSettings);
-
-        public CodeLensFeature(
-            EditorSession editorSession,
-            IMessageHandlers messageHandlers,
-            ILogger logger)
-                : base(logger)
-        {
-            this.editorSession = editorSession;
-
-            messageHandlers.SetRequestHandler(
-                CodeLensRequest.Type,
-                this.HandleCodeLensRequest);
-
-            messageHandlers.SetRequestHandler(
-                CodeLensResolveRequest.Type,
-                this.HandleCodeLensResolveRequest);
-        }
 
+        /// <summary>
+        /// Create a new CodeLens instance around a given editor session
+        /// from the component registry.
+        /// </summary>
+        /// <param name="components">
+        /// The component registry to provider other components and to register the CodeLens provider in.
+        /// </param>
+        /// <param name="editorSession">The editor session context of the CodeLens provider.</param>
+        /// <returns>A new CodeLens provider for the given editor session.</returns>
         public static CodeLensFeature Create(
             IComponentRegistry components,
             EditorSession editorSession)
         {
             var codeLenses =
                 new CodeLensFeature(
                     editorSession,
-                    components.Get<IMessageHandlers>(),
+                    JsonSerializer.Create(Constants.JsonSerializerSettings),
                     components.Get<ILogger>());
 
+            var messageHandlers = components.Get<IMessageHandlers>();
+
+            messageHandlers.SetRequestHandler(
+                CodeLensRequest.Type,
+                codeLenses.HandleCodeLensRequest);
+
+            messageHandlers.SetRequestHandler(
+                CodeLensResolveRequest.Type,
+                codeLenses.HandleCodeLensResolveRequest);
+
             codeLenses.Providers.Add(
                 new ReferencesCodeLensProvider(
                     editorSession));
@@ -67,42 +68,78 @@ public static CodeLensFeature Create(
             return codeLenses;
         }
 
+        /// <summary>
+        /// The editor session context to get workspace and language server data from.
+        /// </summary>
+        private readonly EditorSession _editorSession;
+
+        /// <summary>
+        /// The json serializer instance for CodeLens object translation.
+        /// </summary>
+        private readonly JsonSerializer _jsonSerializer;
+
+        /// <summary>
+        ///
+        /// </summary>
+        /// <param name="editorSession"></param>
+        /// <param name="jsonSerializer"></param>
+        /// <param name="logger"></param>
+        private CodeLensFeature(
+            EditorSession editorSession,
+            JsonSerializer jsonSerializer,
+            ILogger logger)
+                : base(logger)
+        {
+            _editorSession = editorSession;
+            _jsonSerializer = jsonSerializer;
+        }
+
+        /// <summary>
+        /// Get all the CodeLenses for a given script file.
+        /// </summary>
+        /// <param name="scriptFile">The PowerShell script file to get CodeLenses for.</param>
+        /// <returns>All generated CodeLenses for the given script file.</returns>
         public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile)
         {
-            return
-                this.InvokeProviders(p => p.ProvideCodeLenses(scriptFile))
-                    .SelectMany(r => r)
-                    .ToArray();
+            return InvokeProviders(provider => provider.ProvideCodeLenses(scriptFile))
+                .SelectMany(codeLens => codeLens)
+                .ToArray();
         }
 
+        /// <summary>
+        /// Handles a request for CodeLenses from VSCode.
+        /// </summary>
+        /// <param name="codeLensParams">Parameters on the CodeLens request that was received.</param>
+        /// <param name="requestContext"></param>
         private async Task HandleCodeLensRequest(
             CodeLensRequest codeLensParams,
             RequestContext<LanguageServer.CodeLens[]> requestContext)
         {
-            JsonSerializer jsonSerializer =
-                JsonSerializer.Create(
-                    Constants.JsonSerializerSettings);
+            ScriptFile scriptFile = _editorSession.Workspace.GetFile(
+                codeLensParams.TextDocument.Uri);
 
-            var scriptFile =
-                this.editorSession.Workspace.GetFile(
-                    codeLensParams.TextDocument.Uri);
+            CodeLens[] codeLensResults = ProvideCodeLenses(scriptFile);
 
-            var codeLenses =
-                this.ProvideCodeLenses(scriptFile)
-                    .Select(
-                        codeLens =>
-                            codeLens.ToProtocolCodeLens(
-                                new CodeLensData
-                                {
-                                    Uri = codeLens.File.ClientFilePath,
-                                    ProviderId = codeLens.Provider.ProviderId
-                                },
-                                this.jsonSerializer))
-                    .ToArray();
-
-            await requestContext.SendResult(codeLenses);
+            var codeLensResponse = new LanguageServer.CodeLens[codeLensResults.Length];
+            for (int i = 0; i < codeLensResults.Length; i++)
+            {
+                codeLensResponse[i] = codeLensResults[i].ToProtocolCodeLens(
+                    new CodeLensData
+                    {
+                        Uri = codeLensResults[i].File.ClientFilePath,
+                        ProviderId = codeLensResults[i].Provider.ProviderId
+                    },
+                    _jsonSerializer);
+            }
+
+            await requestContext.SendResult(codeLensResponse);
         }
 
+        /// <summary>
+        /// Handle a CodeLens resolve request from VSCode.
+        /// </summary>
+        /// <param name="codeLens">The CodeLens to be resolved/updated.</param>
+        /// <param name="requestContext"></param>
         private async Task HandleCodeLensResolveRequest(
             LanguageServer.CodeLens codeLens,
             RequestContext<LanguageServer.CodeLens> requestContext)
@@ -113,13 +150,13 @@ private async Task HandleCodeLensResolveRequest(
                 CodeLensData codeLensData = codeLens.Data.ToObject<CodeLensData>();
 
                 ICodeLensProvider originalProvider =
-                    this.Providers.FirstOrDefault(
+                    Providers.FirstOrDefault(
                         provider => provider.ProviderId.Equals(codeLensData.ProviderId));
 
                 if (originalProvider != null)
                 {
                     ScriptFile scriptFile =
-                        this.editorSession.Workspace.GetFile(
+                        _editorSession.Workspace.GetFile(
                             codeLensData.Uri);
 
                     ScriptRegion region = new ScriptRegion
@@ -143,7 +180,7 @@ await originalProvider.ResolveCodeLensAsync(
 
                     await requestContext.SendResult(
                         resolvedCodeLens.ToProtocolCodeLens(
-                            this.jsonSerializer));
+                            _jsonSerializer));
                 }
                 else
                 {
@@ -153,6 +190,9 @@ await requestContext.SendError(
             }
         }
 
+        /// <summary>
+        /// Represents data expected back in an LSP CodeLens response.
+        /// </summary>
         private class CodeLensData
         {
             public string Uri { get; set; }
diff --git a/src/PowerShellEditorServices.Host/CodeLens/PesterCodeLensProvider.cs b/src/PowerShellEditorServices.Host/CodeLens/PesterCodeLensProvider.cs
@@ -15,71 +15,90 @@ namespace Microsoft.PowerShell.EditorServices.CodeLenses
 {
     internal class PesterCodeLensProvider : FeatureProviderBase, ICodeLensProvider
     {
-        private static char[] QuoteChars = new char[] { '\'', '"'};
+        /// <summary>
+        /// The editor session context to provide CodeLenses for.
+        /// </summary>
+        private EditorSession _editorSession;
 
-        private EditorSession editorSession;
-        private IDocumentSymbolProvider symbolProvider;
+        /// <summary>
+        /// The symbol provider to get symbols from to build code lenses with.
+        /// </summary>
+        private IDocumentSymbolProvider _symbolProvider;
 
+        /// <summary>
+        /// Create a new Pester CodeLens provider for a given editor session.
+        /// </summary>
+        /// <param name="editorSession">The editor session context for which to provide Pester CodeLenses.</param>
         public PesterCodeLensProvider(EditorSession editorSession)
         {
-            this.editorSession = editorSession;
-            this.symbolProvider = new PesterDocumentSymbolProvider();
+            _editorSession = editorSession;
+            _symbolProvider = new PesterDocumentSymbolProvider();
         }
 
-        private IEnumerable<CodeLens> GetPesterLens(
+        /// <summary>
+        /// Get the Pester CodeLenses for a given Pester symbol.
+        /// </summary>
+        /// <param name="pesterSymbol">The Pester symbol to get CodeLenses for.</param>
+        /// <param name="scriptFile">The script file the Pester symbol comes from.</param>
+        /// <returns>All CodeLenses for the given Pester symbol.</returns>
+        private CodeLens[] GetPesterLens(
             PesterSymbolReference pesterSymbol,
             ScriptFile scriptFile)
         {
-            var clientCommands = new ClientCommand[]
+            var codeLensResults = new CodeLens[]
             {
-                new ClientCommand(
-                    "PowerShell.RunPesterTests",
-                    "Run tests",
-                    new object[]
-                    {
-                        scriptFile.ClientFilePath,
-                        false, // Don't debug
-                        pesterSymbol.TestName,
-                    }),
+                new CodeLens(
+                    this,
+                    scriptFile,
+                    pesterSymbol.ScriptRegion,
+                    new ClientCommand(
+                        "PowerShell.RunPesterTests",
+                        "Run tests",
+                        new object[] { scriptFile.ClientFilePath, false /* No debug */, pesterSymbol.TestName })),
 
-                new ClientCommand(
-                    "PowerShell.RunPesterTests",
-                    "Debug tests",
-                    new object[]
-                    {
-                        scriptFile.ClientFilePath,
-                        true, // Run in debugger
-                        pesterSymbol.TestName,
-                    }),
+                new CodeLens(
+                    this,
+                    scriptFile,
+                    pesterSymbol.ScriptRegion,
+                    new ClientCommand(
+                        "PowerShell.RunPesterTests",
+                        "Debug tests",
+                        new object[] { scriptFile.ClientFilePath, true /* Run in debugger */, pesterSymbol.TestName })),
             };
 
-            return
-                clientCommands.Select(
-                    command =>
-                        new CodeLens(
-                            this,
-                            scriptFile,
-                            pesterSymbol.ScriptRegion,
-                            command));
+            return codeLensResults;
         }
 
+        /// <summary>
+        /// Get all Pester CodeLenses for a given script file.
+        /// </summary>
+        /// <param name="scriptFile">The script file to get Pester CodeLenses for.</param>
+        /// <returns>All Pester CodeLenses for the given script file.</returns>
         public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile)
         {
-            var symbols =
-                this.symbolProvider
-                    .ProvideDocumentSymbols(scriptFile);
+            var lenses = new List<CodeLens>();
+            foreach (SymbolReference symbol in _symbolProvider.ProvideDocumentSymbols(scriptFile))
+            {
+                if (symbol is PesterSymbolReference pesterSymbol)
+                {
+                    if (pesterSymbol.Command != PesterCommandType.Describe)
+                    {
+                        continue;
+                    }
 
-            var lenses =
-                symbols
-                    .OfType<PesterSymbolReference>()
-                    .Where(s => s.Command == PesterCommandType.Describe)
-                    .SelectMany(s => this.GetPesterLens(s, scriptFile))
-                    .Where(codeLens => codeLens != null)
-                    .ToArray();
+                    lenses.AddRange(GetPesterLens(pesterSymbol, scriptFile));
+                }
+            }
 
-            return lenses;
+            return lenses.ToArray();
         }
 
+        /// <summary>
+        /// Resolve the CodeLens provision asynchronously -- just wraps the CodeLens argument in a task.
+        /// </summary>
+        /// <param name="codeLens">The code lens to resolve.</param>
+        /// <param name="cancellationToken"></param>
+        /// <returns>The given CodeLens, wrapped in a task.</returns>
         public Task<CodeLens> ResolveCodeLensAsync(
             CodeLens codeLens,
             CancellationToken cancellationToken)
diff --git a/src/PowerShellEditorServices.Host/CodeLens/ReferencesCodeLensProvider.cs b/src/PowerShellEditorServices.Host/CodeLens/ReferencesCodeLensProvider.cs
