Repository navigation
Preserve KSP metadata context during delayed cache updates - #49
Merged
Merged
Conversation
Summary: Intent: - Prevent redundant KotlinKsp execution after resource-only edits when the KSP execution inputs are unchanged. Changes: - Retain the originating KSP InputMetadataProvider and ArtifactExpander in immutable delayed-action context. KotlinCompile's action-scoped metadata does not contain all KSP tool inputs; using it for the deferred KSP cache update corrupts the stored input fingerprint. - Replay the KSP context when KotlinCompile completes the delayed local action-cache update. Preserve invalidation for actual KSP tool changes. - Add regression coverage through the enabled tracker/callback path with disjoint action-scoped metadata providers. No rules_kotlin change or resource-avoidance-policy change is required. Test Plan: - Regression target: //src/test/java/com/google/devtools/build/lib/actions:ActionsTests, filtered to com.google.devtools.build.lib.actions.ActionCacheCheckerTest#delayedKspCacheUpdate.*. The three new cases fail on the stock base at the intended cache/checksum assertions and pass with this fix. Full ActionCacheCheckerTest class: 51 cases, zero failures. Tool-change controls first establish a healthy cache hit, then mutate ksp2_invoker.jar or symbol-processing.jar and assert DIFFERENT_FILES. - Real Android replay on jbarr-j2k: use locally compiled stock/patched Bazel 8.8.0 via USE_LOCAL_BAZEL=1 and the same locally built rules_kotlin release archive. Seed //libraries/foundation/ui/uview-injection:src_release in arm64/Dagger context with actual KSP/Kotlin workers, then make an XML value edit. Stock unnecessarily executes one KSP worker; patched uses the local cache (zero KSP workers). All selected KSP execution inputs/argv/environment/platform/outputs match; 24 project resource jars are byte-identical. Duration: 5.639s to 2.431s. - Paired Helix //apps/presidio/helix/app-apk:bin_debug first-value-edit replay: 1430.416s to 1149.037s (281.379s / 19.671% shorter); KSP workers 1557 to 0; Javac workers 5658 to 5659. Same source, rules archive and Java/Android tools; 24 jobs, pinned CPUs, same RAM/worker caps, shrink off, current unused_classes policy. Network/disk cache fallback disabled during measured edits. Warmup excluded (different cache sources). This is one paired affected incremental build, not a clean-build or fleet-wide estimate. - Builds were serialized, test telemetry/shared-cache uploads disabled, and the Android checkout restored. No whole-Bazel-suite claim. Revert Plan: Revert this PR. Existing malformed local cache entries may incur one final miss when revalidated; no global cache expunge is required.
azad-uber-2
reviewed
Oct 20, 2026
Comment on lines
+307
to
+310
| delayedCachedActionsData.put( | ||
| getKey(action), | ||
| new DelayedCachedActionData( | ||
| action, token, inputMetadataProvider, outputMetadataStore, artifactExpander)); |
There was a problem hiding this comment.
Each pending entry now also holds KSP's full input metadata. If the matching KotlinCompile never runs (cache hit, failure, interrupted build), the entry stays in memory until KSP runs again for that target. Should we clear this map at the end of each build?
Collaborator
Author
There was a problem hiding this comment.
it already is, since the whole tracker is thrown away when the build ends
There was a problem hiding this comment.
Great! I assumed it was kept after the build
Comment on lines
893
to
+898
| updateActionCache( | ||
| delayedActionData.action, | ||
| delayedActionData.token, | ||
| inputMetadataProvider, | ||
| delayedActionData.inputMetadataProvider, | ||
| delayedActionData.outputMetadataStore, | ||
| artifactExpander, | ||
| delayedActionData.artifactExpander, |
There was a problem hiding this comment.
This reads KSP's metadata after KSP has already finished. Is that still safe to use at this point?
Collaborator
Author
There was a problem hiding this comment.
yes, it's safe since KSP's input data is saved before KSP runs and never changes afterward. reading it later gives the same values as reading it right away
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.
Summary
Fix unnecessary
KotlinKspexecution caused by incorrect metadata during deferred local action-cache updates.When
KotlinCompilecompletes, Bazel updates the earlier KSP cache entry using KotlinCompile’s input metadata provider. That provider lacks KSP-specific tool inputs, corrupting the stored fingerprint and causing an unnecessary cache miss on the next revalidation.InputMetadataProviderandArtifactExpanderin immutable delayed-action context.rules_kotlinchange or resource-invalidation policy change is required.Test Plan
Regression coverage
ActionCacheCheckerTestclass passes: 51 cases, zero failures.ksp2_invoker.jarorsymbol-processing.jarand verify invalidation withDIFFERENT_FILES.Android integration
Validated on
jbarr-j2kusing locally compiled stock/patched Bazel 8.8.0 and the same locally builtrules_kotlinarchive.The full-app result is one paired affected incremental build, not a clean-build or fleet-wide estimate. Measured edits used identical resource policies and worker limits, with network/disk cache fallback disabled. Warmup times were excluded.
Builds were serialized, telemetry and shared-cache uploads were disabled, and the Android checkout was restored afterward.
Revert Plan
Revert this PR.
No cache expunge or data migration is required. After applying the fix, existing malformed local cache entries may incur one final miss when first revalidated; subsequent updates preserve the correct KSP metadata.