Skip to content

Commit

Permalink
Rebuild fixes
Browse files Browse the repository at this point in the history
This fixes the following issues in Rebuild validation

1. Adds back `<PlatformTarget>AnyCPU</PlatformTarget>` 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: dotnet#52800
  • Loading branch information
jaredpar committed Mar 9, 2023
1 parent ac14e43 commit c8d155d
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 41 deletions.
1 change: 1 addition & 0 deletions Compilers.slnf
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/Tools/BuildValidator/BuildValidator.csproj
Expand Up @@ -8,6 +8,7 @@
<Nullable>enable</Nullable>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsShipping>false</IsShipping>
<PlatformTarget>AnyCPU</PlatformTarget>
</PropertyGroup>

<ItemGroup Label="Project References">
Expand Down
2 changes: 1 addition & 1 deletion src/Tools/BuildValidator/CompilationDiff.cs
Expand Up @@ -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}");
Expand Down
106 changes: 67 additions & 39 deletions src/Tools/BuildValidator/LocalReferenceResolver.cs
Expand Up @@ -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;

Expand All @@ -29,6 +30,12 @@ internal class LocalReferenceResolver
/// </summary>
private readonly Dictionary<Guid, AssemblyInfo> _mvidMap = new();

/// <summary>
/// Map file names to all of the paths it exists at. This map is depopulated as we realize
/// the information from these file locations.
/// </summary>
private readonly Dictionary<string, List<string>> _nameToLocationsMap = new();

/// <summary>
/// This maps a given file name to all of the <see cref="AssemblyInfo"/> that we ever considered
/// for that file name. It's useful for diagnostic purposes to see where we may have missed a
Expand All @@ -38,24 +45,49 @@ internal class LocalReferenceResolver
private readonly HashSet<DirectoryInfo> _indexDirectories = new();
private readonly ILogger _logger;

public LocalReferenceResolver(Options options, ILoggerFactory loggerFactory)
private LocalReferenceResolver(Dictionary<string, List<string>> nameToLocationsMap, ILogger logger)
{
_nameToLocationsMap = nameToLocationsMap;
_logger = logger;
}

public static LocalReferenceResolver Create(Options options, ILoggerFactory loggerFactory)
{
_logger = loggerFactory.CreateLogger<LocalReferenceResolver>();
var logger = loggerFactory.CreateLogger<LocalReferenceResolver>();
var directories = new List<DirectoryInfo>();
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<string, List<string>>();
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()
Expand Down Expand Up @@ -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<AssemblyInfo>();
_nameToLocationsMap.Remove(fileName);

foreach (var directory in _indexDirectories)
using var _ = _logger.BeginScope($"Populating {fileName}");
var assemblyInfoList = new List<AssemblyInfo>();
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;
}
}
}
3 changes: 2 additions & 1 deletion src/Tools/BuildValidator/Program.cs
Expand Up @@ -195,7 +195,7 @@ static IEnumerable<string> getAssemblyPaths(string directory)
private static bool ValidateFiles(IEnumerable<AssemblyInfo> assemblyInfos, Options options, ILoggerFactory loggerFactory)
{
var logger = loggerFactory.CreateLogger<Program>();
var referenceResolver = new LocalReferenceResolver(options, loggerFactory);
var referenceResolver = LocalReferenceResolver.Create(options, loggerFactory);

var assembliesCompiled = new List<CompilationDiff>();
foreach (var assemblyInfo in assemblyInfos)
Expand Down Expand Up @@ -315,6 +315,7 @@ private static bool ValidateFiles(IEnumerable<AssemblyInfo> assemblyInfos, Optio
}
catch (Exception ex)
{
logger.LogError(ex.Message);
return CompilationDiff.CreateMiscError(assemblyInfo, ex.Message);
}
}
Expand Down

0 comments on commit c8d155d

Please sign in to comment.