[8.8.0] Fix cycles when checking the local repo contents cache#29087
[8.8.0] Fix cycles when checking the local repo contents cache#29087fmeum wants to merge 1 commit into
Conversation
|
Thank you for contributing to the Bazel repository! This pull request has been marked as stale since it has not had any activity in the last 30 days. It will be closed in the next 30 days unless any other activity occurs. If you think this PR is still relevant and should stay open, please post any comment here and the PR will no longer be marked as stale. |
|
Thank you for contributing to the Bazel repository! This pull request has been marked as stale since it has not had any activity in the last 30 days. It will be closed in the next 30 days unless any other activity occurs. If you think this PR is still relevant and should stay open, please post any comment here and the PR will no longer be marked as stale. |
|
Do I understand correctly that checking the up-to-dateness of local repo contents cache entries is indeed quadratic, but still okay-ish in practice because Skyframe caches things? |
Fixes bazelbuild#27517 by checking Skyframe deps in batches that stop right before any dep that may cause a cycle if checked while previous deps are out-of-date. This is accompanied by a restructuring of `RepoRecordedInput` that consolidates all Skyframe logic associated with the computation of the corresponding value exclusively within that class. This will also be helpful in adding support for dynamic inputs to the remote repo contents cache in future work. The upstream commit additionally moved the entirety of `RepositoryFetchFunction` to Skyframe workers so that checking the up-to-dateness of local repo contents cache entries isn't quadratic. That worker refactor is omitted in this 8.x backport, which keeps the existing split `RepositoryDelegatorFunction`/`StarlarkRepositoryFunction` architecture and restart-based evaluation; the cycle fix and the `RepoRecordedInput` restructuring are applied on that model. The batched up-to-dateness check is restart-safe (`isAnyValueOutdated` short-circuits while its batch's values are still missing). Closes bazelbuild#28206. Co-authored-by: Xudong Yang <wyverald@gmail.com> PiperOrigin-RevId: 855252657 Change-Id: Ica18760ae79da5155fc0f3d8cd4f24c52a034c86 (cherry picked from commit 72a25a9)
Head branch was pushed to by a user without write access
Yes, that's what I would assume. The marker file is reread every time, which is something we could cache if it shows up as relevant overhead. |
|
Merged in 023a3f6 |
Fixes #27517 by checking Skyframe deps in batches that stop right before any dep that may cause a cycle if checked while previous deps are out-of-date.
This is accompanied by a restructuring of
RepoRecordedInputthat consolidates all Skyframe logic associated with the computation of the corresponding value exclusively within that class. This will also be helpful in adding support for dynamic inputs to the remote repo contents cache in future work.8.x adaptation: the upstream commit additionally moved the entirety of
RepositoryFetchFunctionto Skyframe workers so that checking the up-to-dateness of local repo contents cache entries isn't quadratic. That worker refactor is omitted in this backport: v9 mergedRepositoryDelegatorFunctionandStarlarkRepositoryFunctioninto a single worker-basedRepositoryFetchFunction, which 8.x does not have. This backport keeps 8.x's split, restart-based architecture and applies only the cycle fix and theRepoRecordedInputrestructuring on that model. The batched up-to-dateness check is restart-safe (isAnyValueOutdatedshort-circuits while its batch's values are still missing, and the caller restarts viaenv.valuesMissing()). TheRepoRecordedInputAPI is otherwise ported faithfully so that the dynamic-inputs follow-up (#27634) can be cherry-picked later.Depends on the repo-env handling port (#29069), whose commits are included in this branch.
Closes #28206.
Co-authored-by: Xudong Yang wyverald@gmail.com
PiperOrigin-RevId: 855252657
Change-Id: Ica18760ae79da5155fc0f3d8cd4f24c52a034c86
(cherry picked from commit 72a25a9)