Skip to content

Commit

Permalink
Various perf fixes to minimize linter CPU impact (#3852)
Browse files Browse the repository at this point in the history
* Additional perf follow-ups

* Follow up feedback, optimization to reuse semantic models

* Fixes

* Add tracing to CLI

* Add tracing env variable

* Fix lint error

* Add unit test for semantic model deduplication

* Debug -> Trace

* Log trace messages to MSBuild message output
  • Loading branch information
anthony-c-martin committed Aug 3, 2021
1 parent afe6cb3 commit bdda2d0
Show file tree
Hide file tree
Showing 22 changed files with 411 additions and 82 deletions.
32 changes: 19 additions & 13 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,24 @@
"version": "2.0.0",
"configurations": [
{
"name": "Bicep VSCode Extension",
"name": "VSCode Extension",
"type": "extensionHost",
"request": "launch",
"runtimeExecutable": "${execPath}",
"args": [
"--enable-proposed-api",
"--extensionDevelopmentPath=${workspaceRoot}/src/vscode-bicep"
],
"env": {
"BICEP_TRACING_ENABLED": "true"
},
"stopOnEntry": false,
"sourceMaps": true,
"outFiles": ["${workspaceRoot}/out/src/**/*.js"],
"preLaunchTask": "Build VSIX"
},
{
"name": "Bicep Playground",
"type": "node",
"request": "launch",
"program": "${workspaceFolder}/src/playground/node_modules/webpack/bin/webpack.js",
"args": ["serve"],
"cwd": "${workspaceFolder}/src/playground",
"autoAttachChildProcesses": true,
"stopOnEntry": false,
"preLaunchTask": "Build WASM for Playground"
},
{
"name": "Bicep CLI",
"name": "CLI",
"type": "coreclr",
"request": "launch",
"preLaunchTask": "Build CLI",
Expand All @@ -36,10 +28,24 @@
"build",
"${file}"
],
"env": {
"BICEP_TRACING_ENABLED": "true"
},
"cwd": "${workspaceFolder}/src/Bicep.Cli",
"console": "internalConsole",
"stopAtEntry": false
},
{
"name": "Playground",
"type": "node",
"request": "launch",
"program": "${workspaceFolder}/src/playground/node_modules/webpack/bin/webpack.js",
"args": ["serve"],
"cwd": "${workspaceFolder}/src/playground",
"autoAttachChildProcesses": true,
"stopOnEntry": false,
"preLaunchTask": "Build WASM for Playground"
},
{
"name": "Textmate Tests",
"type": "node",
Expand Down
8 changes: 7 additions & 1 deletion src/Bicep.Cli/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using System;
using System.Diagnostics;
using System.Runtime;

namespace Bicep.Cli
Expand Down Expand Up @@ -44,7 +45,12 @@ public static int Main(string[] args)

public int Run(string[] args)
{
ServiceProvider serviceProvider = ConfigureServices();
var serviceProvider = ConfigureServices();

if (bool.TryParse(Environment.GetEnvironmentVariable("BICEP_TRACING_ENABLED"), out var enableTracing) && enableTracing)
{
Trace.Listeners.Add(new TextWriterTraceListener(this.invocationContext.OutputWriter));
}

try
{
Expand Down
29 changes: 29 additions & 0 deletions src/Bicep.Cli/TextWriterTraceListener.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
using System.Diagnostics;
using System.IO;

namespace Bicep.Cli
{
public class TextWriterTraceListener : TraceListener
{
private readonly TextWriter textWriter;

public TextWriterTraceListener(TextWriter textWriter)
{
this.textWriter = textWriter;
}

public override void Write(string? message)
{
textWriter.WriteLine($"TRACE: {message}");
}

public override void WriteLine(string? message)
{
textWriter.WriteLine($"TRACE: {message}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,23 @@ public override IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model)

public static IEnumerable<(TextSpan RelativeSpan, string Value)> FindHostnameMatches(string hostname, string srcText)
{
bool isExactDomainMatch(int index)
// This code is executed VERY frequently, so we need to be ultra-careful making changes here.
// For a compilation, runtime is ~O(number_of_modules * avg_number_of_viable_string_tokens * number_of_hostnames).
// In the language service, we recompile on every keypress.

bool hasLeadingOrTrailingAlphaNumericChar(int index)
{
var leadingAlnum = index > 0 &&
char.IsLetterOrDigit(srcText[index - 1]);
var trailingAlnum = index + hostname.Length < srcText.Length &&
char.IsLetterOrDigit(srcText[index + hostname.Length]);
if (index > 0 && char.IsLetterOrDigit(srcText[index - 1]))
{
return true;
}

if (index + hostname.Length < srcText.Length && char.IsLetterOrDigit(srcText[index + hostname.Length]))
{
return true;
}

return !leadingAlnum && !trailingAlnum;
return false;
}

if (hostname.Length == 0)
Expand All @@ -91,7 +100,7 @@ bool isExactDomainMatch(int index)
}

// check preceding and trailing chars to verify we're not dealing with a substring
if (isExactDomainMatch(matchIndex))
if (!hasLeadingOrTrailingAlphaNumericChar(matchIndex))
{
var matchText = srcText.Substring(matchIndex, hostname.Length);
yield return (RelativeSpan: new TextSpan(matchIndex, hostname.Length), Value: matchText);
Expand Down Expand Up @@ -170,6 +179,20 @@ public override void VisitStringSyntax(StringSyntax syntax)
}
base.VisitStringSyntax(syntax);
}

public override void VisitResourceDeclarationSyntax(ResourceDeclarationSyntax syntax)
{
// Skip resource 'type' strings - there's no reason to perform analysis on them
this.VisitNodes(syntax.LeadingNodes);
this.Visit(syntax.Value);
}

public override void VisitModuleDeclarationSyntax(ModuleDeclarationSyntax syntax)
{
// Skip resource 'path' strings - there's no reason to perform analysis on them
this.VisitNodes(syntax.LeadingNodes);
this.Visit(syntax.Value);
}
}
}
}
8 changes: 8 additions & 0 deletions src/Bicep.Core/Extensions/EnumerableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source)
}
}
}

public static ILookup<T, T> InvertLookup<T>(this ILookup<T, T> source)
=> source.SelectMany(group => group.Select(val => (group.Key, val)))
.ToLookup(x => x.val, x => x.Key);

public static ImmutableDictionary<TKey, ImmutableHashSet<TValue>> ToImmutableDictionary<TKey, TValue>(this ILookup<TKey, TValue> source)
where TKey : notnull
=> source.ToImmutableDictionary(x => x.Key, x => x.ToImmutableHashSet());
}
}

3 changes: 3 additions & 0 deletions src/Bicep.Core/Semantics/ArmTemplateSemanticModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Azure.Deployments.Core.Definitions.Schema;
using Azure.Deployments.Core.Entities;
Expand All @@ -24,6 +25,8 @@ public class ArmTemplateSemanticModel : ISemanticModel

public ArmTemplateSemanticModel(ArmTemplateFile sourceFile)
{
Trace.WriteLine($"Building semantic model for {sourceFile.FileUri}");

this.SourceFile = sourceFile;

this.targetScopeLazy = new(() =>
Expand Down
17 changes: 10 additions & 7 deletions src/Bicep.Core/Semantics/Compilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using Bicep.Core.Configuration;
using Bicep.Core.Diagnostics;
using Bicep.Core.Extensions;
using Bicep.Core.TypeSystem;
using Bicep.Core.Workspaces;

Expand All @@ -15,18 +16,20 @@ public class Compilation
{
private readonly ImmutableDictionary<ISourceFile, Lazy<ISemanticModel>> lazySemanticModelLookup;

public Compilation(IResourceTypeProvider resourceTypeProvider, SourceFileGrouping sourceFileGrouping)
public Compilation(IResourceTypeProvider resourceTypeProvider, SourceFileGrouping sourceFileGrouping, ImmutableDictionary<ISourceFile, ISemanticModel>? modelLookup = null)
{
this.SourceFileGrouping = sourceFileGrouping;
this.ResourceTypeProvider = resourceTypeProvider;
this.lazySemanticModelLookup = sourceFileGrouping.SourceFiles.ToImmutableDictionary(
sourceFile => sourceFile,
sourceFile => new Lazy<ISemanticModel>(() => sourceFile switch
{
BicepFile bicepFile => new SemanticModel(this, bicepFile, SourceFileGrouping.FileResolver),
ArmTemplateFile armTemplateFile => new ArmTemplateSemanticModel(armTemplateFile),
_ => throw new ArgumentOutOfRangeException(nameof(sourceFile)),
}));
sourceFile => (modelLookup is not null && modelLookup.TryGetValue(sourceFile, out var existingModel)) ?
new(existingModel) :
new Lazy<ISemanticModel>(() => sourceFile switch
{
BicepFile bicepFile => new SemanticModel(this, bicepFile, SourceFileGrouping.FileResolver),
ArmTemplateFile armTemplateFile => new ArmTemplateSemanticModel(armTemplateFile),
_ => throw new ArgumentOutOfRangeException(nameof(sourceFile)),
}));
}

public SourceFileGrouping SourceFileGrouping { get; }
Expand Down
3 changes: 3 additions & 0 deletions src/Bicep.Core/Semantics/SemanticModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Bicep.Core.Analyzers.Linter;
using Bicep.Core.Configuration;
Expand Down Expand Up @@ -32,6 +33,8 @@ public class SemanticModel : ISemanticModel

public SemanticModel(Compilation compilation, BicepFile sourceFile, IFileResolver fileResolver)
{
Trace.WriteLine($"Building semantic model for {sourceFile.FileUri}");

Compilation = compilation;
SourceFile = sourceFile;
FileResolver = fileResolver;
Expand Down
28 changes: 28 additions & 0 deletions src/Bicep.Core/Workspaces/SourceFileGrouping.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using Bicep.Core.Diagnostics;
using Bicep.Core.FileSystem;
using Bicep.Core.Syntax;
using System.Linq;
using static Bicep.Core.Diagnostics.DiagnosticBuilder;

namespace Bicep.Core.Workspaces
Expand All @@ -16,19 +18,23 @@ public class SourceFileGrouping
BicepFile entryPoint,
ImmutableHashSet<ISourceFile> sourceFiles,
ImmutableDictionary<ModuleDeclarationSyntax, ISourceFile> sourceFilesByModuleDeclaration,
ImmutableDictionary<ISourceFile, ImmutableHashSet<ISourceFile>> sourceFileParentLookup,
ImmutableDictionary<ModuleDeclarationSyntax, ErrorBuilderDelegate> errorBuildersByModuleDeclaration,
ImmutableHashSet<ModuleDeclarationSyntax> modulesToRestore)
{
this.FileResolver = fileResolver;
this.EntryPoint = entryPoint;
this.SourceFiles = sourceFiles;
this.SourceFilesByModuleDeclaration = sourceFilesByModuleDeclaration;
this.SourceFileParentLookup = sourceFileParentLookup;
this.ErrorBuildersByModuleDeclaration = errorBuildersByModuleDeclaration;
this.ModulesToRestore = modulesToRestore;
}

public ImmutableDictionary<ModuleDeclarationSyntax, ISourceFile> SourceFilesByModuleDeclaration { get; }

public ImmutableDictionary<ISourceFile, ImmutableHashSet<ISourceFile>> SourceFileParentLookup { get; }

public ImmutableDictionary<ModuleDeclarationSyntax, ErrorBuilderDelegate> ErrorBuildersByModuleDeclaration { get; }

public ImmutableHashSet<ModuleDeclarationSyntax> ModulesToRestore { get; }
Expand All @@ -42,6 +48,28 @@ public class SourceFileGrouping
public ISourceFile LookUpModuleSourceFile(ModuleDeclarationSyntax moduleDeclaration) =>
this.SourceFilesByModuleDeclaration[moduleDeclaration];

public ImmutableHashSet<ISourceFile> GetFilesDependingOn(ISourceFile sourceFile)
{
var filesToCheck = new Queue<ISourceFile>(new [] { sourceFile });
var knownFiles = new HashSet<ISourceFile>();

while (filesToCheck.TryDequeue(out var current))
{
knownFiles.Add(current);

if (SourceFileParentLookup.TryGetValue(current, out var parents))
{
foreach (var parent in parents.Where(x => !knownFiles.Contains(x)))
{
knownFiles.Add(parent);
filesToCheck.Enqueue(parent);
}
}
}

return knownFiles.ToImmutableHashSet();
}

public bool TryLookUpModuleErrorDiagnostic(ModuleDeclarationSyntax moduleDeclaration, [NotNullWhen(true)] out ErrorDiagnostic? errorDiagnostic)
{
if (this.ErrorBuildersByModuleDeclaration.TryGetValue(moduleDeclaration, out var errorBuilder))
Expand Down
16 changes: 10 additions & 6 deletions src/Bicep.Core/Workspaces/SourceFileGroupingBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Immutable;
using System.Linq;
using Bicep.Core.Diagnostics;
using Bicep.Core.Extensions;
using Bicep.Core.FileSystem;
using Bicep.Core.Parsing;
using Bicep.Core.Registry;
Expand Down Expand Up @@ -97,13 +98,14 @@ private SourceFileGrouping Build(Uri entryFileUri)
throw new InvalidOperationException($"Expected {nameof(PopulateRecursive)} to return a BicepFile.");
}

this.ReportFailuresForCycles();
var sourceFileDependencies = this.ReportFailuresForCycles();

return new SourceFileGrouping(
fileResolver,
entryPoint,
sourceFilesByUri.Values.ToImmutableHashSet(),
sourceFilesByModuleDeclaration.ToImmutableDictionary(),
sourceFileDependencies.InvertLookup().ToImmutableDictionary(),
errorBuildersByModuleDeclaration.ToImmutableDictionary(),
modulesToInit.ToImmutableHashSet());
}
Expand Down Expand Up @@ -215,7 +217,7 @@ private ISourceFile AddSourceFile(Uri fileUri, string fileContents, bool isEntry
return sourceFile;
}

private void ReportFailuresForCycles()
private ILookup<ISourceFile, ISourceFile> ReportFailuresForCycles()
{
var sourceFileGraph = this.sourceFilesByUri.Values
.SelectMany(sourceFile => GetModuleDeclarations(sourceFile)
Expand All @@ -226,20 +228,22 @@ private void ReportFailuresForCycles()
.ToLookup(x => x.sourceFile, x => x.referencedFile);

var cycles = CycleDetector<ISourceFile>.FindCycles(sourceFileGraph);
foreach (var kvp in sourceFilesByModuleDeclaration)
foreach (var (moduleDeclaration, moduleSourceFile) in sourceFilesByModuleDeclaration)
{
if (cycles.TryGetValue(kvp.Value, out var cycle))
if (cycles.TryGetValue(moduleSourceFile, out var cycle))
{
if (cycle.Length == 1)
{
errorBuildersByModuleDeclaration[kvp.Key] = x => x.CyclicModuleSelfReference();
errorBuildersByModuleDeclaration[moduleDeclaration] = x => x.CyclicModuleSelfReference();
}
else
{
errorBuildersByModuleDeclaration[kvp.Key] = x => x.CyclicModule(cycle.Select(x => x.FileUri.LocalPath));
errorBuildersByModuleDeclaration[moduleDeclaration] = x => x.CyclicModule(cycle.Select(x => x.FileUri.LocalPath));
}
}
}

return sourceFileGraph;
}

private static IEnumerable<ModuleDeclarationSyntax> GetModuleDeclarations(ISourceFile sourceFile) => sourceFile is BicepFile bicepFile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static BicepCompilationManager CreateCompilationManager(DocumentUri docum

var document = CreateMockDocument(p => receivedParams = p);
var server = CreateMockServer(document);
BicepCompilationManager bicepCompilationManager = new BicepCompilationManager(server.Object, CreateEmptyCompilationProvider(), new Workspace(), BicepCompilationManagerHelper.CreateMockScheduler().Object);
BicepCompilationManager bicepCompilationManager = new BicepCompilationManager(server.Object, CreateEmptyCompilationProvider(), new Workspace(), new FileResolver(), BicepCompilationManagerHelper.CreateMockScheduler().Object);

if (upsertCompilation)
{
Expand Down
Loading

0 comments on commit bdda2d0

Please sign in to comment.