From c8d155dfa6be34baa330f064115421e10a3f0a67 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Wed, 8 Mar 2023 16:19:58 -0800 Subject: [PATCH] Rebuild fixes This fixes the following issues in Rebuild validation 1. Adds back `AnyCPU` so the exe runs under x64 in CI. Running in x86 in CI is putting us right up against the memory boundary and sometimes results in crashes. 2. Changes `LocalReferenceResolver` such that directories are only enumerated twice (once for exe and one for dll). In the past every new name request caused the entire directory set to be walked again. This change saved ~8 seconds on our CI runs. 3. When there is a misc error record it in the build artifacts instead of silently failing. Related: #52800 --- Compilers.slnf | 1 + .../BuildValidator/BuildValidator.csproj | 1 + src/Tools/BuildValidator/CompilationDiff.cs | 2 +- .../BuildValidator/LocalReferenceResolver.cs | 106 +++++++++++------- src/Tools/BuildValidator/Program.cs | 3 +- 5 files changed, 72 insertions(+), 41 deletions(-) diff --git a/Compilers.slnf b/Compilers.slnf index b543ac0c43dce..362ada764c18e 100644 --- a/Compilers.slnf +++ b/Compilers.slnf @@ -68,6 +68,7 @@ "src\\Tools\\Source\\CompilerGeneratorTools\\Source\\IOperationGenerator\\CompilersIOperationGenerator.csproj", "src\\Tools\\Source\\CompilerGeneratorTools\\Source\\VisualBasicSyntaxGenerator\\VisualBasicSyntaxGenerator.vbproj", "src\\Tools\\Source\\CompilerGeneratorTools\\Source\\VisualBasicErrorFactsGenerator\\VisualBasicErrorFactsGenerator.vbproj", + "src\\Tools\\BuildValidator\\BuildValidator.csproj", "src\\Tools\\Source\\RunTests\\RunTests.csproj", "src\\Tools\\TestDiscoveryWorker\\TestDiscoveryWorker.csproj", "src\\Workspaces\\CSharp\\Portable\\Microsoft.CodeAnalysis.CSharp.Workspaces.csproj", diff --git a/src/Tools/BuildValidator/BuildValidator.csproj b/src/Tools/BuildValidator/BuildValidator.csproj index 1a6876b1b3de1..15ac191689414 100644 --- a/src/Tools/BuildValidator/BuildValidator.csproj +++ b/src/Tools/BuildValidator/BuildValidator.csproj @@ -8,6 +8,7 @@ enable true false + AnyCPU diff --git a/src/Tools/BuildValidator/CompilationDiff.cs b/src/Tools/BuildValidator/CompilationDiff.cs index 70d5534c1acd9..a8d24aca794c2 100644 --- a/src/Tools/BuildValidator/CompilationDiff.cs +++ b/src/Tools/BuildValidator/CompilationDiff.cs @@ -216,7 +216,7 @@ public unsafe void WriteArtifacts(string debugPath, ILogger logger) writeMissingReferences(); break; case RebuildResult.MiscError: - // No artifacts to write here + File.WriteAllText(Path.Combine(debugPath, "error.txt"), MiscErrorMessage); break; default: throw new Exception($"Unexpected value {Result}"); diff --git a/src/Tools/BuildValidator/LocalReferenceResolver.cs b/src/Tools/BuildValidator/LocalReferenceResolver.cs index 4958110830330..0dbc92763d1d3 100644 --- a/src/Tools/BuildValidator/LocalReferenceResolver.cs +++ b/src/Tools/BuildValidator/LocalReferenceResolver.cs @@ -13,6 +13,7 @@ using System.Reflection.PortableExecutable; using System.Threading.Tasks; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeGen; using Microsoft.CodeAnalysis.Rebuild; using Microsoft.Extensions.Logging; @@ -29,6 +30,12 @@ internal class LocalReferenceResolver /// private readonly Dictionary _mvidMap = new(); + /// + /// Map file names to all of the paths it exists at. This map is depopulated as we realize + /// the information from these file locations. + /// + private readonly Dictionary> _nameToLocationsMap = new(); + /// /// This maps a given file name to all of the that we ever considered /// for that file name. It's useful for diagnostic purposes to see where we may have missed a @@ -38,24 +45,49 @@ internal class LocalReferenceResolver private readonly HashSet _indexDirectories = new(); private readonly ILogger _logger; - public LocalReferenceResolver(Options options, ILoggerFactory loggerFactory) + private LocalReferenceResolver(Dictionary> nameToLocationsMap, ILogger logger) + { + _nameToLocationsMap = nameToLocationsMap; + _logger = logger; + } + + public static LocalReferenceResolver Create(Options options, ILoggerFactory loggerFactory) { - _logger = loggerFactory.CreateLogger(); + var logger = loggerFactory.CreateLogger(); + var directories = new List(); foreach (var path in options.AssembliesPaths) { - _indexDirectories.Add(new DirectoryInfo(path)); + directories.Add(new DirectoryInfo(path)); } - _indexDirectories.Add(GetNugetCacheDirectory()); + + directories.Add(GetNugetCacheDirectory()); foreach (var path in options.ReferencesPaths) { - _indexDirectories.Add(new DirectoryInfo(path)); + directories.Add(new DirectoryInfo(path)); } - using var _ = _logger.BeginScope("Assembly Reference Search Paths"); - foreach (var directory in _indexDirectories) + using var _ = logger.BeginScope("Assembly Location Cache Population"); + var map = new Dictionary>(); + foreach (var directory in directories) { - _logger.LogInformation($@"""{directory.FullName}"""); + logger.LogInformation($"Searching {directory.FullName}"); + var allFiles = directory + .EnumerateFiles("*.dll", SearchOption.AllDirectories) + .Concat(directory.EnumerateFiles("*.exe", SearchOption.AllDirectories)); + + foreach (var fileInfo in allFiles) + { + if (!map.TryGetValue(fileInfo.Name, out var list)) + { + list = new(); + map[fileInfo.Name] = list; + } + + list.Add(fileInfo.FullName); + } } + + return new LocalReferenceResolver(map, logger); } public static DirectoryInfo GetNugetCacheDirectory() @@ -111,50 +143,46 @@ public bool TryResolveReferences(MetadataReferenceInfo metadataReferenceInfo, [N public bool TryGetAssemblyInfo(MetadataReferenceInfo metadataReferenceInfo, [NotNullWhen(true)] out AssemblyInfo? assemblyInfo) { - if (_mvidMap.TryGetValue(metadataReferenceInfo.ModuleVersionId, out assemblyInfo)) - { - return true; - } + EnsureCachePopulated(metadataReferenceInfo.FileName); + return _mvidMap.TryGetValue(metadataReferenceInfo.ModuleVersionId, out assemblyInfo); + } - if (_nameMap.TryGetValue(metadataReferenceInfo.FileName, out var _)) + private void EnsureCachePopulated(string fileName) + { + if (!_nameToLocationsMap.TryGetValue(fileName, out var list)) { - // The file name of this reference has already been searched for and none of them - // had the correct MVID (else the _mvidMap lookup would succeed). No reason to do - // more work here. - return false; + return; } - var list = new List(); + _nameToLocationsMap.Remove(fileName); - foreach (var directory in _indexDirectories) + using var _ = _logger.BeginScope($"Populating {fileName}"); + var assemblyInfoList = new List(); + foreach (var filePath in list) { - foreach (var fileInfo in directory.EnumerateFiles(metadataReferenceInfo.FileName, SearchOption.AllDirectories)) + if (Util.GetPortableExecutableInfo(filePath) is not { } peInfo) { - if (Util.GetPortableExecutableInfo(fileInfo.FullName) is not { } peInfo) - { - _logger.LogWarning($@"Could not read MVID from ""{fileInfo.FullName}"""); - continue; - } + _logger.LogWarning($@"Could not read MVID from ""{filePath}"""); + continue; + } - if (peInfo.IsReadyToRun) - { - _logger.LogInformation($@"Skipping ReadyToRun image ""{fileInfo.FullName}"""); - continue; - } + if (peInfo.IsReadyToRun) + { + _logger.LogInformation($@"Skipping ReadyToRun image ""{filePath}"""); + continue; + } - var currentInfo = new AssemblyInfo(fileInfo.FullName, peInfo.Mvid); - list.Add(currentInfo); + var currentInfo = new AssemblyInfo(filePath, peInfo.Mvid); + assemblyInfoList.Add(currentInfo); - if (!_mvidMap.ContainsKey(peInfo.Mvid)) - { - _logger.LogTrace($"Caching [{peInfo.Mvid}, {fileInfo.FullName}]"); - _mvidMap[peInfo.Mvid] = currentInfo; - } + if (!_mvidMap.ContainsKey(peInfo.Mvid)) + { + _logger.LogTrace($"Caching [{peInfo.Mvid}, {filePath}]"); + _mvidMap[peInfo.Mvid] = currentInfo; } } - _nameMap[metadataReferenceInfo.FileName] = list; - return _mvidMap.TryGetValue(metadataReferenceInfo.ModuleVersionId, out assemblyInfo); + _nameMap[fileName] = assemblyInfoList; } } } diff --git a/src/Tools/BuildValidator/Program.cs b/src/Tools/BuildValidator/Program.cs index 6effc03da8c94..92bed15247ab4 100644 --- a/src/Tools/BuildValidator/Program.cs +++ b/src/Tools/BuildValidator/Program.cs @@ -195,7 +195,7 @@ static IEnumerable getAssemblyPaths(string directory) private static bool ValidateFiles(IEnumerable assemblyInfos, Options options, ILoggerFactory loggerFactory) { var logger = loggerFactory.CreateLogger(); - var referenceResolver = new LocalReferenceResolver(options, loggerFactory); + var referenceResolver = LocalReferenceResolver.Create(options, loggerFactory); var assembliesCompiled = new List(); foreach (var assemblyInfo in assemblyInfos) @@ -315,6 +315,7 @@ private static bool ValidateFiles(IEnumerable assemblyInfos, Optio } catch (Exception ex) { + logger.LogError(ex.Message); return CompilationDiff.CreateMiscError(assemblyInfo, ex.Message); } }