Rebuild fixes #67240
Rebuild fixes #67240
Conversation
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
|
@dotnet/roslyn-compiler, @RikkiGibson, @jjonescz PTAL |
|
|
||
| foreach (var directory in _indexDirectories) | ||
| using var _ = _logger.BeginScope($"Populating {fileName}"); | ||
| var assemblyInfoList = new List<AssemblyInfo>(); |
There was a problem hiding this comment.
nit: would passing a size hint be helpful here?
There was a problem hiding this comment.
Usually I only pass a size hint when I know what the size will be. In this case it's hard to know. If you watch the output you'll notice that there are hundreds and hundreds of R2R images that end up getting searched. Using capacity here would end up causing the list to overallocate for a lot of System.* DLLs.
|
|
||
| _nameMap[metadataReferenceInfo.FileName] = list; | ||
| return _mvidMap.TryGetValue(metadataReferenceInfo.ModuleVersionId, out assemblyInfo); | ||
| _nameMap[fileName] = assemblyInfoList; |
There was a problem hiding this comment.
I guess the reason we don't do this at the same time we enumerate directories is, this step is more expensive and we don't expect to be asked about every last .exe or .dll? Is that right?
There was a problem hiding this comment.
That's correct. Enumerating the directories is expensive so we try to do it the minimum number of times. Essentially it's cheaper to enumerate *.dll once and record the locations vs. enumerate fileName.dll for each individual request. The enumeration itself is very expensive given the number of dirs that we search so do it once and save.
But delay actually reading the files off disk until we're asked to do so cause we end up indexing a lot more files than we actually need to read.
ghost
added this to the
4 participants
This fixes the following issues in Rebuild validation
<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.LocalReferenceResolversuch 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.Related: #52800