Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47c68821-7f08-4ad9-ba70-d8f39e886a45
Enable task result caching through MSBuildTaskCacheDirectory without requiring hardened graph validation, and enforce the declared-I/O invocation contract at execution time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47c68821-7f08-4ad9-ba70-d8f39e886a45
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47c68821-7f08-4ad9-ba70-d8f39e886a45
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 566e0c9a-761c-4a9c-9d1e-ebc9854fdfb8
Use the engine module identity in cache keys and remove per-entry locking. Publish entries atomically, quarantine invalid entries before replacement, and validate payload metadata from a single open stream. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Hash cached payloads while copying them to output-side staging files, ensuring the installed bytes are the validated bytes while avoiding a second payload read. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Replace cumulative resident-server statistics and their dedicated node packet and CLI commands with four online per-node counters. Emit hit rate and average lookup times through ordinary low-importance logging so summaries remain available in binary logs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Add a configurable 10 GB default size limit with best-effort LRU eviction while preserving an unlimited opt-out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Enable nullable analysis across the task result cache implementation and remove the per-node aggregate statistics summary while retaining per-invocation binlog events. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Restore the existing packet reservation comment and shutdown packet comparison formatting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Replace boilerplate cache response and manifest types with positional readonly record structs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Use TaskParameter and BinaryTranslator for cache-key task items while retaining exact primitive serialization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Use a shared rename-before-delete path for corrupt and evicted entries, and verify replacement publication when cleanup races or fails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Contributor
There was a problem hiding this comment.
The automated expert review could not complete because the review subagent could not access the PR diff through its permitted tools. No findings were posted; this COMMENT is the required fallback and is not an approval.
Warning
Firewall blocked 3 domains
The following domains were blocked by the firewall during workflow execution:
api.github.comgithub.comraw.githubusercontent.com
[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:
tools:
github:
mode: gh-proxySee GitHub Tools for more information on gh-proxy mode.
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "api.github.com"
- "github.com"
- "raw.githubusercontent.com"See Network Configuration for more information.
Generated by Expert Code Review (on open) for #15053 · copilot · gpt56 · 77 AIC · ⌖ 5.43 AIC · ⊞ 21.2K
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate cache correctness and failure-handling issues remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (4)
What changed in this PR
This PR prototypes an opt-in declared-I/O task result cache with persistent storage, input digesting, output restoration, diagnostic replay, lifecycle integration, tests, localization, and documentation.
Changes:
- Adds declared-I/O attributes and metadata discovery.
- Integrates cache lookup, storage, restoration, replay, eviction, and cleanup.
- Adds tests, localized diagnostics, and design documentation.
Review findings:
- Critical (3 votes): Same-length payload corruption can be accepted without integrity verification.
- Moderate (2 votes): Per-output restore failures can leave partially restored filesystem state.
- Moderate (1 vote): Replacement-store failures can leave corrupt entries while reporting success.
- Critical (2 votes): Declared-path probing exceptions can fail the build instead of falling back.
- Critical (3 votes): Digest reuse based only on length and timestamp can produce stale cache hits.
- Moderate (1 vote): Cached diagnostics do not account for effective logging importance.
| File | Summary |
|---|---|
src/Framework/MSBuildDeclaredIOTaskAttribute.cs |
Adds the declared-I/O task attribute. |
src/Framework/MSBuildDeclaredIORequiresUnsetAttribute.cs |
Adds invocation restrictions. |
src/Framework/Loader/LoadedType.cs |
Detects declared-I/O metadata. |
src/Framework.UnitTests/Attribute_Tests.cs |
Tests the new attributes. |
src/Build/Resources/xlf/Strings.zh-Hant.xlf |
Adds Traditional Chinese cache diagnostics. |
src/Build/Resources/xlf/Strings.zh-Hans.xlf |
Adds Simplified Chinese cache diagnostics. |
src/Build/Resources/xlf/Strings.tr.xlf |
Adds Turkish cache diagnostics. |
src/Build/Resources/xlf/Strings.ru.xlf |
Adds Russian cache diagnostics. |
src/Build/Resources/xlf/Strings.pt-BR.xlf |
Adds Brazilian Portuguese cache diagnostics. |
src/Build/Resources/xlf/Strings.pl.xlf |
Adds Polish cache diagnostics. |
src/Build/Resources/xlf/Strings.ko.xlf |
Adds Korean cache diagnostics. |
src/Build/Resources/xlf/Strings.ja.xlf |
Adds Japanese cache diagnostics. |
src/Build/Resources/xlf/Strings.it.xlf |
Adds Italian cache diagnostics. |
src/Build/Resources/xlf/Strings.fr.xlf |
Adds French cache diagnostics. |
src/Build/Resources/xlf/Strings.es.xlf |
Adds Spanish cache diagnostics. |
src/Build/Resources/xlf/Strings.de.xlf |
Adds German cache diagnostics. |
src/Build/Resources/xlf/Strings.cs.xlf |
Adds Czech cache diagnostics. |
src/Build/Resources/Strings.resx |
Adds cache diagnostic messages. |
src/Build/Microsoft.Build.csproj |
Includes cache implementation files. |
src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs |
Exposes loaded task metadata. |
src/Build/BackEnd/Node/OutOfProcNode.cs |
Clears digest state during node cleanup. |
src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs |
Captures task events for replay. |
src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs |
Integrates cache lookup, execution, and storage. |
src/Build/BackEnd/Components/IBuildComponentHost.cs |
Defines the digest-cache component type. |
src/Build/BackEnd/Components/Caching/TaskResultCacheFileDigestCache.cs |
Caches input file digests. |
src/Build/BackEnd/Components/Caching/TaskResultCacheEvent.cs |
Serializes and replays task events. |
src/Build/BackEnd/Components/Caching/TaskResultCache.cs |
Implements cache keys, storage, restoration, and eviction. |
src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs |
Registers the digest-cache factory. |
src/Build/BackEnd/BuildManager/BuildManager.cs |
Clears digest state between builds. |
src/Build.UnitTests/BackEnd/TaskResultCacheDeletion_Tests.cs |
Tests corrupt-entry handling and replacement. |
src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs |
Tests cache behavior and integration. |
documentation/specs/task-result-cache.md |
Documents cache behavior and contracts. |
Comment on lines
+444
to
+457
| if (!TryGetDeclaredPaths( | ||
| taskType, | ||
| task, | ||
| "DeclaredInputs", | ||
| projectDirectory, | ||
| out IReadOnlyList<string>? inputPaths, | ||
| out string? reason) || | ||
| !TryGetDeclaredPaths( | ||
| taskType, | ||
| task, | ||
| "DeclaredOutputs", | ||
| projectDirectory, | ||
| out IReadOnlyList<string>? outputPaths, | ||
| out reason)) |
Comment on lines
+70
to
+73
| FileFingerprint fingerprint = GetFingerprint(path); | ||
| DigestState? state = Volatile.Read(ref _state); | ||
| if (state is not null && state.Fingerprint.Equals(fingerprint)) | ||
| { |
Comment on lines
+392
to
+396
| File.Delete(output.Path); | ||
| File.Move(temporaryFiles[i]!, output.Path); | ||
| temporaryFiles[i] = null; | ||
| File.SetLastWriteTimeUtc(output.Path, DateTime.UtcNow); | ||
| #if NET |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff07a557-5986-410f-90af-d2343672dc1b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a minimal prototype for a cache around Task invocations. The idea is that you opt-in specific tasks by annotating them as "DeclaredIO" (meaning, the only read and write inputs that are declared) and then passing a flag to turn on caching.
I tried to keep this as small as reasonably possible while also providing a lot of useful functionality.
We can use this to compare with other approaches, like full target caching. My prototype implemented support for csc. The other main task that we need to support in runtime is ILLink.