Skip to content

Commit

Permalink
Lazily populate FileIdentifier fields (#69882)
Browse files Browse the repository at this point in the history
* Lazily populate FileIdentifier fields

Found while inspecting profiles for turning on/off razor precise semantic tokens. In the code analysis process, I see 1.6% of CPU and 0.4% of allocations spent in MS.CA.C#.FileIdentifier.Create. (which is over half of the CPU time spent in in CreateSemanticModel). I tested this locally by opening the Roslyn sln, waiting several minutes, and saw over 30K FileIdentifier objects created.

One thing I was curious about was which members were being heavily used in this object as there are differing costs associated with populating each of the members. What I found was that, at least for the scenarios I was testing, none of the objects members were being called for any of the 30K FileIdentifier objects, and thus we could lazily populate this object and not likely ever incur those costs.
  • Loading branch information
ToddGrun authored Sep 14, 2023
1 parent 9ba6a7a commit 638ca0c
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 38 deletions.
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1353,8 +1353,8 @@ private bool IsInScopeOfAssociatedSyntaxTree(Symbol symbol)
return false;
}

var symbolFileIdentifier = ((NamedTypeSymbol)symbol).AssociatedFileIdentifier.GetValueOrDefault();
if (symbolFileIdentifier.FilePathChecksumOpt.IsDefault)
var symbolFileIdentifier = ((NamedTypeSymbol)symbol).AssociatedFileIdentifier;
if (symbolFileIdentifier is null || symbolFileIdentifier.FilePathChecksumOpt.IsDefault)
{
// the containing file of the file-local type has an ill-formed path.
return false;
Expand All @@ -1371,7 +1371,7 @@ FileIdentifier getFileIdentifierForFileTypes()
if (binder is BuckStopsHereBinder lastBinder)
{
// we never expect to bind a file type in a context where the BuckStopsHereBinder lacks an AssociatedFileIdentifier
return lastBinder.AssociatedFileIdentifier.Value;
return lastBinder.AssociatedFileIdentifier ?? throw ExceptionUtilities.Unreachable();
}
}

Expand Down
93 changes: 76 additions & 17 deletions src/Compilers/CSharp/Portable/Symbols/FileIdentifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,98 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Text;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.CSharp;

internal readonly struct FileIdentifier
internal sealed class FileIdentifier
{
private class FileIdentifierData(string? encoderFallbackErrorMessage, string displayFilePath, ImmutableArray<byte> filePathChecksumOpt)
{
public readonly string? EncoderFallbackErrorMessage = encoderFallbackErrorMessage;
public readonly string DisplayFilePath = displayFilePath;
public readonly ImmutableArray<byte> FilePathChecksumOpt = filePathChecksumOpt;
}

private static readonly Encoding s_encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true);

public string? EncoderFallbackErrorMessage { get; init; }
public ImmutableArray<byte> FilePathChecksumOpt { get; init; }
public string DisplayFilePath { get; init; }
private readonly string _filePath;
private FileIdentifierData? _data;

public static FileIdentifier Create(SyntaxTree tree)
=> Create(tree.FilePath);
private FileIdentifier(string filePath)
{
_filePath = filePath;
}

public static FileIdentifier Create(string filePath)
private FileIdentifier(ImmutableArray<byte> filePathChecksumOpt, string displayFilePath)
{
_data = new FileIdentifierData(encoderFallbackErrorMessage: null, displayFilePath, filePathChecksumOpt);
_filePath = string.Empty;
}

[MemberNotNull(nameof(_data))]
private void EnsureInitialized()
{
if (_data is null)
{
string? encoderFallbackErrorMessage = null;
ImmutableArray<byte> hash = default;
try
{
var encodedFilePath = s_encoding.GetBytes(_filePath);
using var hashAlgorithm = SourceHashAlgorithms.CreateDefaultInstance();
hash = hashAlgorithm.ComputeHash(encodedFilePath).ToImmutableArray();
}
catch (EncoderFallbackException ex)
{
encoderFallbackErrorMessage = ex.Message;
}

var displayFilePath = GeneratedNames.GetDisplayFilePath(_filePath);

_data = new FileIdentifierData(encoderFallbackErrorMessage, displayFilePath, hash);
}
}

public string DisplayFilePath
{
string? encoderFallbackErrorMessage = null;
ImmutableArray<byte> hash = default;
try
get
{
var encodedFilePath = s_encoding.GetBytes(filePath);
using var hashAlgorithm = SourceHashAlgorithms.CreateDefaultInstance();
hash = hashAlgorithm.ComputeHash(encodedFilePath).ToImmutableArray();
EnsureInitialized();

return _data.DisplayFilePath;
}
catch (EncoderFallbackException ex)
}

public string? EncoderFallbackErrorMessage
{
get
{
encoderFallbackErrorMessage = ex.Message;
EnsureInitialized();

return _data.EncoderFallbackErrorMessage;
}
}

var displayFilePath = GeneratedNames.GetDisplayFilePath(filePath);
return new FileIdentifier { EncoderFallbackErrorMessage = encoderFallbackErrorMessage, FilePathChecksumOpt = hash, DisplayFilePath = displayFilePath };
public ImmutableArray<byte> FilePathChecksumOpt
{
get
{
EnsureInitialized();

return _data.FilePathChecksumOpt;
}
}

public static FileIdentifier Create(SyntaxTree tree)
=> Create(tree.FilePath);

public static FileIdentifier Create(string filePath)
=> new FileIdentifier(filePath);

public static FileIdentifier Create(ImmutableArray<byte> filePathChecksumOpt, string displayFilePath)
=> new FileIdentifier(filePathChecksumOpt, displayFilePath);
}
Original file line number Diff line number Diff line change
Expand Up @@ -398,15 +398,15 @@ internal abstract override bool MangleName
}

internal sealed override bool IsFileLocal => _lazyUncommonProperties is { lazyFilePathChecksum: { IsDefault: false }, lazyDisplayFileName: { } };
internal sealed override FileIdentifier? AssociatedFileIdentifier
internal sealed override FileIdentifier AssociatedFileIdentifier
{
get
{
// `lazyFilePathChecksum` and `lazyDisplayFileName` of `_lazyUncommonProperties` are initialized in the constructor, not on demand.
// Therefore we can use `_lazyUncommonProperties` directly to avoid additional computations.
// Also important, that computing full uncommon properties here may lead to stack overflow if there is a circular dependency between types in the metadata.
return _lazyUncommonProperties is { lazyFilePathChecksum: { IsDefault: false } checksum, lazyDisplayFileName: { } displayFileName }
? new FileIdentifier { FilePathChecksumOpt = checksum, DisplayFilePath = displayFileName }
? FileIdentifier.Create(checksum, displayFileName)
: null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ public sealed override bool AreLocalsZeroed
}

internal override bool IsFileLocal => _underlyingType.IsFileLocal;
internal override FileIdentifier? AssociatedFileIdentifier => _underlyingType.AssociatedFileIdentifier;
internal override FileIdentifier AssociatedFileIdentifier => _underlyingType.AssociatedFileIdentifier;

internal sealed override NamedTypeSymbol AsNativeInteger() => throw ExceptionUtilities.Unreachable();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ internal override IEnumerable<CSharpAttributeData> GetCustomAttributesToEmit(PEM
}

internal sealed override bool IsFileLocal => _underlyingType.IsFileLocal;
internal sealed override FileIdentifier? AssociatedFileIdentifier => _underlyingType.AssociatedFileIdentifier;
internal sealed override FileIdentifier AssociatedFileIdentifier => _underlyingType.AssociatedFileIdentifier;

internal sealed override NamedTypeSymbol AsNativeInteger() => throw ExceptionUtilities.Unreachable();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public SynthesizedEmbeddedAttributeSymbolBase(
internal override bool MangleName => false;

internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override FileIdentifier AssociatedFileIdentifier => null;

internal override bool HasCodeAnalysisEmbeddedAttribute => true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ internal override bool MangleName
}

internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override FileIdentifier AssociatedFileIdentifier => null;

public override ImmutableArray<TypeParameterSymbol> TypeParameters
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4367,26 +4367,26 @@ void M(C c)
Assert.Equal("C@<tree 0>", type.ToTestDisplayString());
var identifier = type.GetSymbol()!.AssociatedFileIdentifier;
Assert.NotNull(identifier);
AssertEx.Equal(expectedChecksum, identifier.GetValueOrDefault().FilePathChecksumOpt);
Assert.Empty(identifier.GetValueOrDefault().DisplayFilePath);
AssertEx.Equal(expectedChecksum, identifier.FilePathChecksumOpt);
Assert.Empty(identifier.DisplayFilePath);
Assert.True(type.IsFileLocal);

var referencingMetadataComp = CreateCompilation("", new[] { comp.ToMetadataReference() });
type = ((Compilation)referencingMetadataComp).GetTypeByMetadataName("<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C")!;
Assert.Equal("C@<tree 0>", type.ToTestDisplayString());
identifier = type.GetSymbol()!.AssociatedFileIdentifier;
Assert.NotNull(identifier);
AssertEx.Equal(expectedChecksum, identifier.GetValueOrDefault().FilePathChecksumOpt);
Assert.Empty(identifier.GetValueOrDefault().DisplayFilePath);
AssertEx.Equal(expectedChecksum, identifier.FilePathChecksumOpt);
Assert.Empty(identifier.DisplayFilePath);
Assert.True(type.IsFileLocal);

var referencingImageComp = CreateCompilation("", new[] { comp.EmitToImageReference() });
type = ((Compilation)referencingImageComp).GetTypeByMetadataName("<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C")!;
Assert.Equal("C@<unknown>", type.ToTestDisplayString());
identifier = type.GetSymbol()!.AssociatedFileIdentifier;
Assert.NotNull(identifier);
AssertEx.Equal(expectedChecksum, identifier.GetValueOrDefault().FilePathChecksumOpt);
Assert.Empty(identifier.GetValueOrDefault().DisplayFilePath);
AssertEx.Equal(expectedChecksum, identifier.FilePathChecksumOpt);
Assert.Empty(identifier.DisplayFilePath);
Assert.False(type.IsFileLocal);
}

Expand Down Expand Up @@ -4436,8 +4436,8 @@ void M(C<int> c)
Assert.NotNull(identifier);
AssertEx.Equal(
new byte[] { 0xE3, 0xB0, 0xC4, 0x42, 0x98, 0xFC, 0x1C, 0x14, 0x9A, 0xFB, 0xF4, 0xC8, 0x99, 0x6F, 0xB9, 0x24, 0x27, 0xAE, 0x41, 0xE4, 0x64, 0x9B, 0x93, 0x4C, 0xA4, 0x95, 0x99, 0x1B, 0x78, 0x52, 0xB8, 0x55 },
identifier.GetValueOrDefault().FilePathChecksumOpt);
Assert.Empty(identifier.GetValueOrDefault().DisplayFilePath);
identifier.FilePathChecksumOpt);
Assert.Empty(identifier.DisplayFilePath);
Assert.True(type.IsFileLocal);
}

Expand Down Expand Up @@ -4526,8 +4526,8 @@ void M2()
Assert.IsType<RetargetingNamedTypeSymbol>(retargeted);
Assert.False(retargeted.GetPublicSymbol().IsFileLocal);

var originalFileIdentifier = classC1.AssociatedFileIdentifier!.Value;
var retargetedFileIdentifier = retargeted.AssociatedFileIdentifier!.Value;
var originalFileIdentifier = classC1.AssociatedFileIdentifier!;
var retargetedFileIdentifier = retargeted.AssociatedFileIdentifier!;
Assert.Equal(originalFileIdentifier.DisplayFilePath, retargetedFileIdentifier.DisplayFilePath);
Assert.Equal((IEnumerable<byte>)originalFileIdentifier.FilePathChecksumOpt, (IEnumerable<byte>)retargetedFileIdentifier.FilePathChecksumOpt);
Assert.Equal(originalFileIdentifier.EncoderFallbackErrorMessage, retargetedFileIdentifier.EncoderFallbackErrorMessage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ internal CompilationContext(
// looking up file-local types. If there is no document name, use an invalid FilePathChecksumOpt.
FileIdentifier fileIdentifier = methodDebugInfo.ContainingDocumentName is { } documentName
? FileIdentifier.Create(documentName)
: new FileIdentifier { EncoderFallbackErrorMessage = null, FilePathChecksumOpt = ImmutableArray<byte>.Empty, DisplayFilePath = string.Empty };
: FileIdentifier.Create(filePathChecksumOpt: ImmutableArray<byte>.Empty, displayFilePath: string.Empty);

NamespaceBinder = CreateBinderChain(
Compilation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ internal override bool MangleName
}

internal override bool IsFileLocal => false;
internal override FileIdentifier? AssociatedFileIdentifier => null;
internal override FileIdentifier AssociatedFileIdentifier => null;

public override IEnumerable<string> MemberNames
{
Expand Down

0 comments on commit 638ca0c

Please sign in to comment.