Skip to content

Commit ba63de7

Browse files
DustinCampbellDustinCampbell
authored
Handful of performance fixes (#11399)
This PR consists of a handful of performance fixes in the both the compiler and tooling to try and improve the Speedometer regression in Razor completion. I recommend reviewing commit-by-commit. The specifics of each change is described in each commit. CI build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2622490&view=results Test insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/603874
2 parents 66fdab1 + 0c00564 commit ba63de7

File tree

23 files changed

+252
-289
lines changed

23 files changed

+252
-289
lines changed

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorProjectEngineIntegrationTest.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public void Process_GetsImportsFromFeature()
6060
// Arrange
6161
var projectItem = new TestRazorProjectItem("Index.cshtml");
6262

63-
var testImport = Mock.Of<RazorProjectItem>(i => i.Read() == new MemoryStream() && i.FilePath == "testvalue" && i.Exists == true);
63+
var testImport = new TestRazorProjectItem("testvalue");
6464
var importFeature = new Mock<IImportProjectFeature>();
6565
importFeature
6666
.Setup(feature => feature.GetImports(It.IsAny<RazorProjectItem>()))
@@ -85,13 +85,13 @@ public void Process_GetsImportsFromFeature_MultipleFeatures()
8585
// Arrange
8686
var projectItem = new TestRazorProjectItem("Index.cshtml");
8787

88-
var testImport1 = Mock.Of<RazorProjectItem>(i => i.Read() == new MemoryStream() && i.FilePath == "testvalue1" && i.Exists == true);
88+
var testImport1 = new TestRazorProjectItem("testvalue1");
8989
var importFeature1 = new Mock<IImportProjectFeature>();
9090
importFeature1
9191
.Setup(feature => feature.GetImports(It.IsAny<RazorProjectItem>()))
9292
.Returns(new[] { testImport1 });
9393

94-
var testImport2 = Mock.Of<RazorProjectItem>(i => i.Read() == new MemoryStream() && i.FilePath == "testvalue2" && i.Exists == true);
94+
var testImport2 = new TestRazorProjectItem("testvalue2");
9595
var importFeature2 = new Mock<IImportProjectFeature>();
9696
importFeature2
9797
.Setup(feature => feature.GetImports(It.IsAny<RazorProjectItem>()))

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/RazorProjectEngineTest.cs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,12 @@ public void GetImportSourceDocuments_DoesNotIncludeNonExistentItems()
142142
public void GetImportSourceDocuments_UnreadableItem_Throws()
143143
{
144144
// Arrange
145-
var projectItem = new Mock<RazorProjectItem>(MockBehavior.Strict);
146-
projectItem.SetupGet(p => p.Exists).Returns(true);
147-
projectItem.SetupGet(p => p.PhysicalPath).Returns("path/to/file.cshtml");
148-
projectItem.Setup(p => p.Read()).Throws(new IOException("Couldn't read file."));
149-
using PooledArrayBuilder<RazorProjectItem> items = [projectItem.Object];
145+
var projectItem = new TestRazorProjectItem(
146+
filePath: "path/to/file.cshtml",
147+
physicalPath: "path/to/file.cshtml",
148+
relativePhysicalPath: "path/to/file.cshtml",
149+
onRead: () => throw new IOException("Couldn't read file."));
150+
using PooledArrayBuilder<RazorProjectItem> items = [projectItem];
150151

151152
// Act & Assert
152153
var exception = Assert.Throws<IOException>(() => RazorProjectEngine.GetImportSourceDocuments(in items));
@@ -157,13 +158,12 @@ public void GetImportSourceDocuments_UnreadableItem_Throws()
157158
public void GetImportSourceDocuments_WithSuppressExceptions_UnreadableItem_DoesNotThrow()
158159
{
159160
// Arrange
160-
var projectItem = new Mock<RazorProjectItem>(MockBehavior.Strict);
161-
projectItem.SetupGet(p => p.Exists).Returns(true);
162-
projectItem.SetupGet(p => p.PhysicalPath).Returns("path/to/file.cshtml");
163-
projectItem.SetupGet(p => p.FilePath).Returns("path/to/file.cshtml");
164-
projectItem.SetupGet(p => p.RelativePhysicalPath).Returns("path/to/file.cshtml");
165-
projectItem.Setup(p => p.Read()).Throws(new IOException("Couldn't read file."));
166-
using PooledArrayBuilder<RazorProjectItem> items = [projectItem.Object];
161+
var projectItem = new TestRazorProjectItem(
162+
filePath: "path/to/file.cshtml",
163+
physicalPath: "path/to/file.cshtml",
164+
relativePhysicalPath: "path/to/file.cshtml",
165+
onRead: () => throw new IOException("Couldn't read file."));
166+
using PooledArrayBuilder<RazorProjectItem> items = [projectItem];
167167

168168
// Act
169169
var sourceDocuments = RazorProjectEngine.GetImportSourceDocuments(in items, suppressExceptions: true);

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentImportProjectFeature.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ private sealed class ComponentImportProjectItem : RazorProjectItem
5353
{
5454
public static readonly ComponentImportProjectItem Instance = new();
5555

56+
private static RazorSourceDocument? s_source;
57+
5658
private ComponentImportProjectItem()
5759
{
5860
}
@@ -72,5 +74,8 @@ private ComponentImportProjectItem()
7274
public override string FileKind => FileKinds.ComponentImport;
7375

7476
public override Stream Read() => s_fileContent.CreateStream();
77+
78+
internal override RazorSourceDocument GetSource()
79+
=> s_source ?? InterlockedOperations.Initialize(ref s_source, base.GetSource());
7580
}
7681
}

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorProjectFileSystem.cs

Lines changed: 85 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.IO;
7-
using System.Linq;
88

99
namespace Microsoft.AspNetCore.Razor.Language;
1010

@@ -14,7 +14,25 @@ public DefaultRazorProjectFileSystem(string root)
1414
{
1515
ArgHelper.ThrowIfNullOrEmpty(root);
1616

17-
Root = root.Replace('\\', '/').TrimEnd('/');
17+
// If "/" is passed in, we want that to be the value of root. We don't want root to end up
18+
// as an empty string.
19+
if (root == DefaultBasePath)
20+
{
21+
Root = DefaultBasePath;
22+
}
23+
else
24+
{
25+
root = root.Replace('\\', '/').TrimEnd('/');
26+
27+
// Was the entire string just repeated '\' and '/' characters? If so, that's an invalid path.
28+
// Just throw instead of setting Root to an empty string.
29+
if (root.Length == 0)
30+
{
31+
ThrowHelper.ThrowArgumentException(nameof(root), $"Invalid path provided.");
32+
}
33+
34+
Root = root;
35+
}
1836
}
1937

2038
public string Root { get; }
@@ -23,67 +41,98 @@ public override IEnumerable<RazorProjectItem> EnumerateItems(string basePath)
2341
{
2442
var absoluteBasePath = NormalizeAndEnsureValidPath(basePath);
2543

26-
var directory = new DirectoryInfo(absoluteBasePath);
27-
if (!directory.Exists)
44+
if (!Directory.Exists(absoluteBasePath))
2845
{
29-
return [];
46+
yield break;
3047
}
3148

32-
return directory
33-
.EnumerateFiles("*.cshtml", SearchOption.AllDirectories)
34-
.Concat(directory.EnumerateFiles("*.razor", SearchOption.AllDirectories))
35-
.Select(file =>
36-
{
37-
var relativePhysicalPath = file.FullName.Substring(absoluteBasePath.Length + 1); // Include leading separator
38-
var filePath = "/" + relativePhysicalPath.Replace(Path.DirectorySeparatorChar, '/');
49+
foreach (var filePath in Directory.EnumerateFiles(absoluteBasePath, "*.cshtml", SearchOption.AllDirectories))
50+
{
51+
yield return CreateItem(filePath, fileKind: null, basePath, absoluteBasePath);
52+
}
3953

40-
return new DefaultRazorProjectItem(basePath, filePath, relativePhysicalPath, fileKind: null, file, cssScope: null);
41-
});
54+
foreach (var filePath in Directory.EnumerateFiles(absoluteBasePath, "*.razor", SearchOption.AllDirectories))
55+
{
56+
yield return CreateItem(filePath, fileKind: null, basePath, absoluteBasePath);
57+
}
4258
}
4359

4460
public override RazorProjectItem GetItem(string path, string? fileKind)
4561
{
46-
var absoluteBasePath = NormalizeAndEnsureValidPath("/");
62+
var absoluteBasePath = Root;
4763
var absolutePath = NormalizeAndEnsureValidPath(path);
4864

49-
var file = new FileInfo(absolutePath);
5065
if (!absolutePath.StartsWith(absoluteBasePath, StringComparison.OrdinalIgnoreCase))
5166
{
52-
throw new InvalidOperationException($"The file '{absolutePath}' is not a descendent of the base path '{absoluteBasePath}'.");
67+
return ThrowHelper.ThrowInvalidOperationException<RazorProjectItem>($"The file '{absolutePath}' is not a descendent of the base path '{absoluteBasePath}'.");
5368
}
5469

55-
var relativePhysicalPath = file.FullName.Substring(absoluteBasePath.Length + 1); // Include leading separator
70+
return CreateItem(absolutePath, fileKind, DefaultBasePath, absoluteBasePath);
71+
}
72+
73+
private static DefaultRazorProjectItem CreateItem(string path, string? fileKind, string basePath, string absoluteBasePath)
74+
{
75+
var physicalPath = Path.GetFullPath(path);
76+
var relativePhysicalPath = physicalPath[(absoluteBasePath.Length + 1)..]; // Don't include leading separator
77+
5678
var filePath = "/" + relativePhysicalPath.Replace(Path.DirectorySeparatorChar, '/');
5779

58-
return new DefaultRazorProjectItem("/", filePath, relativePhysicalPath, fileKind, new FileInfo(absolutePath), cssScope: null);
80+
return new DefaultRazorProjectItem(basePath, filePath, physicalPath, relativePhysicalPath, fileKind, cssScope: null);
5981
}
6082

6183
protected override string NormalizeAndEnsureValidPath(string path)
6284
{
85+
// PERF: If we're asked to normalize "/", there's no need to compare and manipulate strings to
86+
// ultimately return the value of Root.
87+
if (path == DefaultBasePath)
88+
{
89+
return Root;
90+
}
91+
6392
ArgHelper.ThrowIfNullOrEmpty(path);
6493

65-
var absolutePath = path.Replace('\\', '/');
94+
var normalizedPath = path.Replace('\\', '/');
6695

67-
// Check if the given path is an absolute path. It is absolute if,
68-
// 1. It starts with Root or
69-
// 2. It is a network share path and starts with a '//'. Eg. //servername/some/network/folder
70-
if (!absolutePath.StartsWith(Root, StringComparison.OrdinalIgnoreCase) &&
71-
!absolutePath.StartsWith("//", StringComparison.OrdinalIgnoreCase))
96+
// Check if the given path is an absolute path. It is absolute if...
97+
//
98+
// 1. It is a network share path and starts with a '//' (e.g. //server/some/network/folder) or...
99+
// 2. It starts with Root
100+
if (normalizedPath is ['/', '/', ..] ||
101+
normalizedPath.StartsWith(Root, StringComparison.OrdinalIgnoreCase))
72102
{
73-
// This is not an absolute path. Strip the leading slash if any and combine it with Root.
74-
if (path[0] == '/' || path[0] == '\\')
103+
return normalizedPath;
104+
}
105+
106+
// This is not an absolute path, so we combine it with Root to produce the final path.
107+
108+
// If the root doesn't end in a '/', and the path doesn't start with a '/', we'll need to add one.
109+
var needsSlash = Root[^1] is not '/' && normalizedPath[0] is not '/';
110+
var length = Root.Length + normalizedPath.Length + (needsSlash ? 1 : 0);
111+
112+
return StringExtensions.CreateString(
113+
length,
114+
state: (Root, normalizedPath, needsSlash),
115+
static (span, state) =>
75116
{
76-
path = path.Substring(1);
77-
}
117+
var (root, normalizedPath, needsSlash) = state;
78118

79-
// Instead of `C:filename.ext`, we want `C:/filename.ext`.
80-
absolutePath = Root.EndsWith(':') && !path.IsNullOrEmpty()
81-
? Root + "/" + path
82-
: Path.Combine(Root, path);
83-
}
119+
var rootSpan = root.AsSpan();
120+
var pathSpan = normalizedPath.AsSpan();
121+
122+
// Copy the root first.
123+
rootSpan.CopyTo(span);
124+
span = span[rootSpan.Length..];
84125

85-
absolutePath = absolutePath.Replace('\\', '/');
126+
// Add a slash if we need one.
127+
if (needsSlash)
128+
{
129+
span[0] = '/';
130+
span = span[1..];
131+
}
86132

87-
return absolutePath;
133+
// Finally, add the path.
134+
Debug.Assert(span.Length == pathSpan.Length, "The span should be the same length as the path.");
135+
pathSpan.CopyTo(span);
136+
});
88137
}
89138
}

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorProjectItem.cs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,58 @@ namespace Microsoft.AspNetCore.Razor.Language;
99

1010
internal class DefaultRazorProjectItem : RazorProjectItem
1111
{
12+
private FileInfo _fileInfo;
13+
private readonly string _physicalFilePath;
1214
private readonly string _fileKind;
1315

1416
/// <summary>
1517
/// Initializes a new instance of <see cref="DefaultRazorProjectItem"/>.
1618
/// </summary>
1719
/// <param name="basePath">The base path.</param>
18-
/// <param name="relativePhysicalPath">The physical path of the base path.</param>
1920
/// <param name="filePath">The path.</param>
21+
/// <param name="relativePhysicalPath">The physical path of the base path.</param>
2022
/// <param name="fileKind">The file kind. If null, the document kind will be inferred from the file extension.</param>
2123
/// <param name="file">The <see cref="FileInfo"/>.</param>
2224
/// <param name="cssScope">A scope identifier that will be used on elements in the generated class, or null.</param>
2325
public DefaultRazorProjectItem(string basePath, string filePath, string relativePhysicalPath, string fileKind, FileInfo file, string cssScope)
2426
{
2527
BasePath = basePath;
2628
FilePath = filePath;
29+
_physicalFilePath = null;
30+
RelativePhysicalPath = relativePhysicalPath;
31+
_fileKind = fileKind;
32+
_fileInfo = file;
33+
CssScope = cssScope;
34+
}
35+
36+
/// <summary>
37+
/// Initializes a new instance of <see cref="DefaultRazorProjectItem"/>.
38+
/// </summary>
39+
/// <param name="basePath">The base path.</param>
40+
/// <param name="filePath">The path.</param>
41+
/// <param name="physicalPath">The physical path of the file path.</param>
42+
/// <param name="relativePhysicalPath">The physical path of the base path.</param>
43+
/// <param name="fileKind">The file kind. If null, the document kind will be inferred from the file extension.</param>
44+
/// <param name="cssScope">A scope identifier that will be used on elements in the generated class, or null.</param>
45+
public DefaultRazorProjectItem(string basePath, string filePath, string physicalPath, string relativePhysicalPath, string fileKind, string cssScope)
46+
{
47+
BasePath = basePath;
48+
FilePath = filePath;
49+
_physicalFilePath = physicalPath;
2750
RelativePhysicalPath = relativePhysicalPath;
2851
_fileKind = fileKind;
29-
File = file;
3052
CssScope = cssScope;
3153
}
3254

33-
public FileInfo File { get; }
55+
public FileInfo File => _fileInfo ??= new(FilePath);
3456

3557
public override string BasePath { get; }
3658

3759
public override string FilePath { get; }
3860

39-
public override bool Exists => File.Exists;
61+
public override bool Exists => _fileInfo?.Exists ?? System.IO.File.Exists(PhysicalPath);
4062

41-
public override string PhysicalPath => File.FullName;
63+
public override string PhysicalPath => _fileInfo?.FullName ?? _physicalFilePath;
4264

4365
public override string RelativePhysicalPath { get; }
4466

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorProjectEngine.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ private RazorCodeDocument CreateCodeDocumentCore(
147147
Action<RazorParserOptionsBuilder>? configureParser = null,
148148
Action<RazorCodeGenerationOptionsBuilder>? configureCodeGeneration = null)
149149
{
150-
var source = RazorSourceDocument.ReadFrom(projectItem);
150+
var source = projectItem.GetSource();
151151
var importSources = GetImportSources(projectItem, designTime: false);
152152

153153
return CreateCodeDocumentCore(
@@ -193,7 +193,7 @@ private RazorCodeDocument CreateCodeDocumentDesignTimeCore(
193193
Action<RazorParserOptionsBuilder>? configureParser = null,
194194
Action<RazorCodeGenerationOptionsBuilder>? configureCodeGeneration = null)
195195
{
196-
var source = RazorSourceDocument.ReadFrom(projectItem);
196+
var source = projectItem.GetSource();
197197
var importSources = GetImportSources(projectItem, designTime: true);
198198

199199
return CreateCodeDocumentDesignTimeCore(source, projectItem.FileKind, importSources, tagHelpers: null, configureParser, configureCodeGeneration);
@@ -464,7 +464,7 @@ internal static ImmutableArray<RazorSourceDocument> GetImportSourceDocuments(
464464
try
465465
{
466466
// Normal import, has file paths, content etc.
467-
var sourceDocument = RazorSourceDocument.ReadFrom(importItem);
467+
var sourceDocument = importItem.GetSource();
468468
imports.Add(sourceDocument);
469469
}
470470
catch (IOException) when (suppressExceptions)

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorProjectEngineBuilderExtensions.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ public IReadOnlyList<RazorProjectItem> GetImports(RazorProjectItem projectItem)
326326
private sealed class InMemoryProjectItem : RazorProjectItem
327327
{
328328
private readonly InMemoryFileContent _fileContent;
329+
private RazorSourceDocument _source;
329330

330331
public InMemoryProjectItem(string content)
331332
{
@@ -343,6 +344,9 @@ public InMemoryProjectItem(string content)
343344
public override bool Exists => true;
344345

345346
public override Stream Read() => _fileContent.CreateStream();
347+
348+
internal override RazorSourceDocument GetSource()
349+
=> _source ?? InterlockedOperations.Initialize(ref _source, base.GetSource());
346350
}
347351
}
348352

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorProjectFileSystem.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ namespace Microsoft.AspNetCore.Razor.Language;
1313
/// </summary>
1414
public abstract partial class RazorProjectFileSystem
1515
{
16+
internal const string DefaultBasePath = "/";
17+
1618
public static readonly RazorProjectFileSystem Empty = new EmptyFileSystem();
1719

1820
/// <summary>
@@ -57,7 +59,7 @@ public RazorProjectItem GetItem(string path)
5759
/// </remarks>
5860
public IEnumerable<RazorProjectItem> FindHierarchicalItems(string path, string fileName)
5961
{
60-
return FindHierarchicalItems(basePath: "/", path, fileName);
62+
return FindHierarchicalItems(basePath: DefaultBasePath, path, fileName);
6163
}
6264

6365
/// <summary>

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorProjectItem.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,8 @@ public string FilePathWithoutExtension
132132
}
133133
}
134134

135-
internal RazorSourceDocument RazorSourceDocument { get; set; }
135+
internal virtual RazorSourceDocument GetSource()
136+
=> RazorSourceDocument.ReadFrom(this);
136137

137138
private string DebuggerToString()
138139
{

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorSourceDocument.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,6 @@ public static RazorSourceDocument ReadFrom(RazorProjectItem projectItem)
146146
filePath = projectItem.FilePath;
147147
}
148148

149-
if (projectItem.RazorSourceDocument is not null)
150-
{
151-
return projectItem.RazorSourceDocument;
152-
}
153-
154149
using (var stream = projectItem.Read())
155150
{
156151
// Autodetect the encoding.

0 commit comments

Comments
 (0)