From 53cb673c8b6e164c223d52e33e632b774241837f Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Fri, 3 Dec 2021 15:18:21 -0800 Subject: [PATCH 1/4] Lazy load ISourceLinkService to reduce DLL loads --- .../PdbSourceDocumentLoaderServiceTests.cs | 5 +++-- .../PdbSourceDocumentLoaderService.cs | 12 ++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentLoaderServiceTests.cs b/src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentLoaderServiceTests.cs index e24ab6bd694c3..844e8b58681d1 100644 --- a/src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentLoaderServiceTests.cs +++ b/src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentLoaderServiceTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Immutable; using System.IO; using System.Security.Cryptography; @@ -36,7 +37,7 @@ await RunTestAsync(async path => var sourceFilePath = Path.Combine(path, "SourceLink.cs"); File.Move(GetSourceFilePath(path), sourceFilePath); - var sourceLinkService = new TestSourceLinkService(sourceFilePath: sourceFilePath); + var sourceLinkService = new Lazy(() => new TestSourceLinkService(sourceFilePath: sourceFilePath)); var service = new PdbSourceDocumentLoaderService(sourceLinkService); using var hash = SHA256.Create(); @@ -69,7 +70,7 @@ await RunTestAsync(async path => var sourceFilePath = Path.Combine(path, "SourceLink.cs"); File.Move(GetSourceFilePath(path), sourceFilePath); - var sourceLinkService = new TestSourceLinkService(sourceFilePath: sourceFilePath); + var sourceLinkService = new Lazy(() => new TestSourceLinkService(sourceFilePath: sourceFilePath)); var service = new PdbSourceDocumentLoaderService(sourceLinkService); var sourceDocument = new SourceDocument("goo.cs", Text.SourceHashAlgorithm.None, default, null, SourceLinkUrl: null); diff --git a/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs b/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs index 3466b3d9a9642..c8d47f6464863 100644 --- a/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs +++ b/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs @@ -20,11 +20,15 @@ namespace Microsoft.CodeAnalysis.PdbSourceDocument internal sealed class PdbSourceDocumentLoaderService : IPdbSourceDocumentLoaderService { private const int SourceLinkTimeout = 1000; - private readonly ISourceLinkService? _sourceLinkService; + + /// + /// Lazy import ISourceLinkService because it can cause debugger + /// + private readonly Lazy _sourceLinkService; [ImportingConstructor] [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code")] - public PdbSourceDocumentLoaderService([Import(AllowDefault = true)] ISourceLinkService? sourceLinkService) + public PdbSourceDocumentLoaderService([Import(AllowDefault = true)] Lazy sourceLinkService) { _sourceLinkService = sourceLinkService; } @@ -105,14 +109,14 @@ public PdbSourceDocumentLoaderService([Import(AllowDefault = true)] ISourceLinkS private async Task TryGetSourceLinkFileAsync(SourceDocument sourceDocument, Encoding encoding, IPdbSourceDocumentLogger? logger, CancellationToken cancellationToken) { - if (_sourceLinkService is null || sourceDocument.SourceLinkUrl is null) + if (sourceDocument.SourceLinkUrl is null || _sourceLinkService.Value is null) return null; // This should ideally be the repo-relative path to the file, and come from SourceLink: https://github.com/dotnet/sourcelink/pull/699 var relativePath = Path.GetFileName(sourceDocument.FilePath); var delay = Task.Delay(SourceLinkTimeout, cancellationToken); - var sourceFileTask = _sourceLinkService.GetSourceFilePathAsync(sourceDocument.SourceLinkUrl, relativePath, logger, cancellationToken); + var sourceFileTask = _sourceLinkService.Value.GetSourceFilePathAsync(sourceDocument.SourceLinkUrl, relativePath, logger, cancellationToken); var winner = await Task.WhenAny(sourceFileTask, delay).ConfigureAwait(false); From a8db8ad0cedf67be93d4a07a49ae338c5a65385e Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Sat, 4 Dec 2021 00:51:03 -0800 Subject: [PATCH 2/4] Fix a case where MetadataAsSourceFile was eagerly resolving lazy providers on creation --- .../MetadataAsSource/MetadataAsSourceFileService.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs b/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs index d8b1b7893ff6c..c91803f073398 100644 --- a/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs +++ b/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs @@ -40,13 +40,13 @@ internal class MetadataAsSourceFileService : IMetadataAsSourceFileService private Mutex? _mutex; private string? _rootTemporaryPathWithGuid; private readonly string _rootTemporaryPath; - private readonly ImmutableArray _providers; + private readonly ImmutableArray> _providers; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] public MetadataAsSourceFileService([ImportMany] IEnumerable> providers) { - _providers = ExtensionOrderer.Order(providers).Select(lz => lz.Value).ToImmutableArray(); + _providers = ExtensionOrderer.Order(providers).ToImmutableArray(); _rootTemporaryPath = Path.Combine(Path.GetTempPath(), "MetadataAsSource"); } @@ -90,8 +90,9 @@ public async Task GetGeneratedFileAsync(Project project, I Contract.ThrowIfNull(_workspace); var tempPath = GetRootPathWithGuid_NoLock(); - foreach (var provider in _providers) + foreach (var lazyProvider in _providers) { + var provider = lazyProvider.Value; var providerTempPath = Path.Combine(tempPath, provider.GetType().Name); var result = await provider.GetGeneratedFileAsync(_workspace, project, symbol, signaturesOnly, allowDecompilation, providerTempPath, cancellationToken).ConfigureAwait(false); if (result is not null) @@ -179,8 +180,9 @@ public void CleanupGeneratedFiles() _rootTemporaryPathWithGuid = null; } - foreach (var provider in _providers) + foreach (var lazyProvider in _providers) { + var provider = lazyProvider.Value; provider.CleanupGeneratedFiles(_workspace); } From 97fb07884c4afb4bb89881b0e0c75e6db1d4b8e1 Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Sat, 4 Dec 2021 11:07:04 -0800 Subject: [PATCH 3/4] fix cleanup so it doesn't eager load providers --- .../Portable/MetadataAsSource/MetadataAsSourceFileService.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs b/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs index c91803f073398..93d7f89a2f7c1 100644 --- a/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs +++ b/src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs @@ -180,9 +180,10 @@ public void CleanupGeneratedFiles() _rootTemporaryPathWithGuid = null; } - foreach (var lazyProvider in _providers) + // Only cleanup for providers that have actually generated a file. This keeps us from + // accidentally loading lazy providers on cleanup that weren't used + foreach (var provider in _tempFileToProviderMap.Values.Distinct()) { - var provider = lazyProvider.Value; provider.CleanupGeneratedFiles(_workspace); } From 6aec24fc224ce2694c40b1666fd9edf548cff191 Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Sun, 5 Dec 2021 16:27:18 -0800 Subject: [PATCH 4/4] Update comment --- .../Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs b/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs index c8d47f6464863..0d2a2a43a1bf4 100644 --- a/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs +++ b/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentLoaderService.cs @@ -23,6 +23,7 @@ internal sealed class PdbSourceDocumentLoaderService : IPdbSourceDocumentLoaderS /// /// Lazy import ISourceLinkService because it can cause debugger + /// binaries to be eagerly loaded even if they are never used. /// private readonly Lazy _sourceLinkService;