Repository navigation
Add experimental ILLink task caching and cache maintenance - #135065
Merged
Merged
Conversation
dotnet-policy-service
Bot
added
the
linkable-framework
github-actions
Bot
added
the
area-Tools-ILLink
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @agocke, @dotnet/illink |
sbomer
added this pull request to stack #135070
October 1, 2026 22:43
sbomer
force-pushed
the
sbomer/illink-task-cache
branch
from
October 2, 2026 17:17
d5ed48b to
32042e7
Compare
This was referenced Oct 2, 2026
sbomer
force-pushed
the
sbomer/illink-task-cache
branch
from
October 5, 2026 20:07
db1e592 to
f322c92
Compare
Cache eligible whole-task invocations with environment-only opt-in and explicit timestamp-based purging. Bypass caching for extra arguments; runtime-specific argument support is deferred to the next layer. Assisted-by: Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 561d1c87-e14b-46f8-be7f-a08f544ec537
Assisted-by: Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 561d1c87-e14b-46f8-be7f-a08f544ec537
Assisted-by: Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 561d1c87-e14b-46f8-be7f-a08f544ec537
Replace the write-to-temp + File.Replace + retry approach with a plain overwrite. File.Replace's ReplaceFile API isn't meant for concurrent writers to the same destination, which caused a transient Windows CI failure. A plain File.WriteAllText is safe here: FileShare.Read makes the OS serialize conflicting writers, so a losing writer's open fails before touching the file. The marker is advisory bookkeeping, and CachePurge.cs already tolerates malformed content per entry. Assisted-by: Claude:claude-sonnet-5 (GitHub Copilot CLI) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 561d1c87-e14b-46f8-be7f-a08f544ec537
sbomer
force-pushed
the
sbomer/illink-task-cache
branch
from
October 5, 2026 21:35
f322c92 to
9f2118f
Compare
sbomer
requested review from
a team and
agocke
and
a balanced review from Copilot
October 6, 2026 20:55
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Non-atomic usage-marker refreshes can corrupt timestamps and prevent purge from reclaiming affected entries.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds opt-in caching of complete ILLink MSBuild outputs and a standalone tool for idle-cache maintenance.
Changes:
- Adds content-based cache keys, output restoration, and successful-result storage.
- Tracks usage and purges entries older than an explicit UTC cutoff.
- Adds tests, build integration, and configuration documentation.
Validation was limited to source inspection; builds, tests, and benchmarks were not run.
| File | Description |
|---|---|
src/tools/illink/test/ILLink.Tasks.Tests/Mock.cs |
Exposes cache helpers and captures messages. |
src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.csproj |
Adds isolated-process testing support. |
src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs |
Tests cache behavior and invalidation. |
src/tools/illink/test/ILLink.CacheTool.Tests/ILLink.CacheTool.Tests.csproj |
Adds maintenance test project. |
src/tools/illink/test/ILLink.CacheTool.Tests/CachePurgeTests.cs |
Tests purge rules and command validation. |
src/tools/illink/src/ILLink.Tasks/LinkTask.cs |
Integrates caching into task execution. |
src/tools/illink/src/ILLink.Tasks/ILLinkCacheEntry.cs |
Defines entry names and usage markers. |
src/tools/illink/src/ILLink.Tasks/ILLinkCache.cs |
Implements cache storage and restoration. |
src/tools/illink/src/ILLink.Tasks/ILLink.Tasks.csproj |
Grants test access to internals. |
src/tools/illink/src/ILLink.CacheTool/README.md |
Documents maintenance and installation. |
src/tools/illink/src/ILLink.CacheTool/Program.cs |
Defines the purge command. |
src/tools/illink/src/ILLink.CacheTool/ILLink.CacheTool.csproj |
Packages the maintenance tool. |
src/tools/illink/src/ILLink.CacheTool/CachePurge.cs |
Deletes entries by last-used timestamp. |
src/tools/illink/illink.slnx |
Includes maintenance projects. |
eng/Subsets.props |
Integrates tool and test builds. |
docs/tools/illink/README.md |
Links cache design documentation. |
docs/tools/illink/illink-tasks.md |
Documents configuration and maintenance. |
docs/design/tools/illink/task-cache.md |
Specifies cache behavior and limitations. |
Keep purge usable if a best-effort last-used refresh leaves an invalid marker. Report the fallback while preserving errors for actual maintenance failures. Assisted-by: GitHub Copilot CLI:gpt-6-sol Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 561d1c87-e14b-46f8-be7f-a08f544ec537
2 tasks
This was referenced Oct 7, 2026
jtschuster
approved these changes
Oct 13, 2026
jtschuster
left a comment
jtschuster
left a comment
Member
There was a problem hiding this comment.
Generally looks good to me. I don't love the global default cache location, especially if full paths of input files are part of the hash, but I don't feel strongly about it.
The task execution test already verifies that unsupported ExtraArgs bypass caching. Drop the overlapping key test case and its unused switch arm. Assisted-by: Copilot:gpt-6-sol Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 561d1c87-e14b-46f8-be7f-a08f544ec537
agocke
approved these changes
Oct 14, 2026
agocke
left a comment
agocke
left a comment
Member
There was a problem hiding this comment.
lgtm. i think there might be some toctou concerns and fs perf optimizations but this is good for a v1
Member
Author
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.
Add experimental whole-invocation caching to the ILLink MSBuild task, enabled through
ILLINK_EXPERIMENTAL_CACHEwith an optionalILLINK_EXPERIMENTAL_CACHE_PATHoverride. Reuse successful outputs without running the linker again, with a key covering arguments, paths, input contents, and the task, linker, and host binaries.Keep this initial implementation narrow: extra arguments bypass caching, cache hits do not replay diagnostics, and the on-disk format has no compatibility guarantee. Include a standalone maintenance tool that purges entries last used before an explicit UTC cutoff. There is no automatic eviction during linking, and purge must run only while the cache is idle. Runtime build integration and CI enablement follow separately.
Note that this approach is directly inspired by dotnet/roslyn#82881.