Cache GetFileVersion to avoid redundant per-project version reads in GenerateDepsFile - #55033
Conversation
GetFileVersion re-opened and re-parsed the Win32 version resource of every runtime file for every project. The same framework/package assemblies are referenced by many projects, so this is highly redundant. Add a path + last-write-time cache mirroring the existing assembly-version cache (thread-safe ConcurrentDictionary). Because the cache is static, in -mt (single process) it is shared across all projects in the build, so it removes far more redundant I/O than in multi-process -m (per-worker caches). Measured on OrchardCore.Cms.Web Rebuild (192 deps calls, IL task): GenerateDepsFile avg -mt 59->20ms (~65%), -m 74->64ms (~14%); deps.json byte-identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves build performance in GenerateDepsFile scenarios by caching Win32 file version reads (FileUtilities.GetFileVersion) so that the same runtime file’s version resource is not repeatedly opened and parsed across many projects in a single MSBuild process.
Changes:
- Introduced a static
ConcurrentDictionarycache keyed by file path plus last-write-time UTC for file version lookups. - Updated
GetFileVersionto return cached values when the file timestamp is unchanged, and to refresh the cache when it changes.
| public static Version? GetFileVersion(string? sourcePath) | ||
| { | ||
| if (sourcePath != null) | ||
| { | ||
| var fvi = FileVersionInfo.GetVersionInfo(sourcePath); | ||
| DateTime lastWriteTimeUtc = File.GetLastWriteTimeUtc(sourcePath); | ||
|
|
||
| if (s_fileVersionCache.TryGetValue(sourcePath, out var cacheEntry) | ||
| && lastWriteTimeUtc == cacheEntry.LastKnownWriteTimeUtc) | ||
| { | ||
| return cacheEntry.Version; | ||
| } | ||
|
|
||
| Version? version = null; | ||
| var fvi = FileVersionInfo.GetVersionInfo(sourcePath); | ||
| if (fvi != null) | ||
| { | ||
| return new Version(fvi.FileMajorPart, fvi.FileMinorPart, fvi.FileBuildPart, fvi.FilePrivatePart); | ||
| version = new Version(fvi.FileMajorPart, fvi.FileMinorPart, fvi.FileBuildPart, fvi.FilePrivatePart); | ||
| } | ||
|
|
||
| s_fileVersionCache[sourcePath] = (lastWriteTimeUtc, version); | ||
| return version; |
| private static readonly ConcurrentDictionary<string, (DateTime LastKnownWriteTimeUtc, Version? Version)> s_fileVersionCache | ||
| = new(StringComparer.OrdinalIgnoreCase); |
|
@dsplaisted / @ericstj (hope it's ok to continue to ask you questions on GH) any reason why we didn't cache the file version information before in the GenerateDepsFile task? Just trying to see if there are any hidden risks in doing that. |
|
@ViktorHofer almost certainly just a performance/impact question - we weren't tracking how frequently the operation was done. This kind of metadata caching seems completely reasonable to do - 100% safe in the case of a single build, potentially less safe across builds but still very likely to be so. |
|
/ba-g flaky test run on the linux-amd64 leg, no reason to believe its related to this change. |
Summary
FileUtilities.GetFileVersionre-opens and re-parses the Win32 version resource of the sameruntime files for every project that references them. The same framework/package assemblies
are referenced by many projects, so this is highly redundant I/O and allocation during
GenerateDepsFile(which calls it per resolved runtime file whenIncludeRuntimeFileVersionsis set).
This adds a
path + last-write-timecache, mirroring the existing cache already used forassembly versions (
GetAssemblyVersion). It is aConcurrentDictionary, so it is safeunder multithreaded (
-mt) builds, and the cached entries are immutable.Why it helps
-mtin particularThe cache is static, so in
-mt(single process) it is shared across all projects in thebuild, eliminating far more redundant reads than in multi-process
-m, where each workerprocess keeps its own cache.
Measurements (OrchardCore
Cms.WebRebuild, 192GenerateDepsFilecalls)GenerateDepsFiletask time:-mt≈ −65% (59 → 20 ms avg),-m≈ −14% (74 → 64 ms avg).GetFileVersiondrops out of the hot path.deps.jsonis byte-identical (verified by hash) — pure speedup, no behavior change.Follow-ups (draft)