Skip to content

Reduce allocations in the CodeLens providers #719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 90 additions & 50 deletions src/PowerShellEditorServices.Host/CodeLens/CodeLensFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand All @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More LINQ to remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the one I put back in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, see #719 (comment)

.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)
Expand All @@ -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
Expand All @@ -143,7 +180,7 @@ await originalProvider.ResolveCodeLensAsync(

await requestContext.SendResult(
resolvedCodeLens.ToProtocolCodeLens(
this.jsonSerializer));
_jsonSerializer));
}
else
{
Expand All @@ -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; }
Expand Down
107 changes: 63 additions & 44 deletions src/PowerShellEditorServices.Host/CodeLens/PesterCodeLensProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only calls ProvideDocumentSymbols once and iterates through the result, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, yes. Why do you ask?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spoke to @SeeminglyScience about this. We're good

{
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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

echo: SelectMany vs List > Array

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to leave this one on the basis of avoiding some LINQ closures

}

/// <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)
Expand Down
Loading