Repository navigation
merge queue: checking #12407 on master (ad1170e) - #12454
Closed
mergify[bot] wants to merge 8 commits into
Closed
mergify[bot] wants to merge 8 commits into
mergify[bot] wants to merge 8 commits into
Conversation
With `-j`, every plan node runs in parallel, and with per-component builds each benchmark suite is its own node, so `cabal bench` could run several benchmarks at the same time, from the same package or across a project. Concurrent benchmarks compete for resources and skew each other's results. Add a `benchLock`, created in `rebuildTargets` next to `registerLock` and `cacheLock`, and take it around the bench phase in `buildAndRegisterUnpackedPackage`. Benchmarks now run one at a time, while configuring and building other components stays parallel. The lock is taken outside `timedDelegate`, so `--build-timings` does not count time spent waiting for other benchmarks. Add a cabal-testsuite test with three benchmarks in two packages, run with `-j3`, each of which fails if another benchmark runs at the same time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016nzdeKCYFDCkWsyM6etgkL
Holding a lock while running a benchmark keeps benchmarks from running at the same time, but other components could still be built while a benchmark runs, which skews its results just as much. Instead, the bench phase of a package no longer runs its benchmarks, but records them. Once all the packages of the plan are built (still in parallel), `rebuildTargets` runs the recorded benchmarks one at a time, in plan order, and records their failures in the build outcomes. Unless we keep going after failures, no benchmark is run if a package failed to build, and no more benchmarks are run once one of them failed. See Note [Running benchmarks]. This replaces the `benchLock` introduced by the previous commit. The Sequential test now also checks that nothing is built once the first benchmark started: pkg-b's benchmark depends on a library, so that it is still being built when pkg-a's benchmarks are ready to run. The golden outputs of MultipleBenchmarks and T5309 change accordingly: builds now come before benchmark runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016nzdeKCYFDCkWsyM6etgkL
A benchmark suite is not a package of its own: the units of the install plan are single components, or whole packages when they cannot be built per component. Say so in Note [Running benchmarks], and refer to units rather than packages in the related comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVJmRffSh16Wx8JR7Kd1dC
Each update of the deferred benchmarks is a transaction of its own, and they are only read once all the units are built, so a TVar buys nothing over an IORef updated with atomicModifyIORef'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVJmRffSh16Wx8JR7Kd1dC
…icant - Walk the plan's execution order directly instead of building an intermediate list of benchmarks; give `go` a signature and drop the now-redundant `BuildFailure` annotation. - changelog.d/issue-7557.md: set the PR number, add `significance: significant`, and spell out the user-visible consequences. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176vAefrw8jPKXdh9h1wKuj
The bench phase of a unit now returns its deferred benchmark, which `rebuildTarget` returns alongside the unit's `BuildResult`, so that `InstallPlan.execute` collects it with the other outcomes. `runDeferredBenchmarks` then reads both from the same map, and the `IORef` threaded through the build functions is gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176vAefrw8jPKXdh9h1wKuj
Instead of being returned in a tuple next to the BuildResult, the deferred benchmark of a unit is now a field of its BuildResult, so the build functions keep their `IO BuildResult` type and `runDeferredBenchmarks` works on plain BuildOutcomes. DeferredBenchmark is a newtype over the action, which also gives it the Show instance that BuildResult derives. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176vAefrw8jPKXdh9h1wKuj
5 of 6 tasks
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
2 participants
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.
Note
This is a merge queue draft opened by Mergify. Nobody needs to act on it.
Mergify opened it for batch prominent-andouille-3ydhjg of the
squash-mergequeue, to test #12407 before it merges intomaster.What this draft tests
Mergify checks that #12407 still passes CI on top of
master. CI runs on this draft's code, built in this order:masteratad1170e✨ Where it stands: closed
Pull request #12453 with higher priority has been queued.
Follow the
squash-mergequeue in the dashboardWhat happens next
Metadata for CI tooling, read by
mergify ciRequired conditions of queue rule
squash-mergefor mergecheck-neutral = Validate post jobcheck-skipped = Validate post jobcheck-success = Validate post job#review-threads-unresolved = 0[🛡 GitHub branch protection]github-review-approved[🛡 GitHub branch protection]check-success = Doctest Cabalcheck-neutral = Doctest Cabalcheck-skipped = Doctest Cabalcheck-success = Meta checkscheck-neutral = Meta checkscheck-skipped = Meta checkscheck-success = docs/readthedocs.org:cabalcheck-neutral = docs/readthedocs.org:cabalcheck-skipped = docs/readthedocs.org:cabalcheck-success = fourmolucheck-neutral = fourmolucheck-skipped = fourmolucheck-success = hlintcheck-neutral = hlintcheck-skipped = hlintcheck-success = Bootstrap post jobcheck-neutral = Bootstrap post jobcheck-skipped = Bootstrap post jobcheck-success = whitespacecheck-neutral = whitespacecheck-skipped = whitespacecheck-success = Check sdist post jobcheck-neutral = Check sdist post jobcheck-skipped = Check sdist post jobcheck-success = Changelogscheck-neutral = Changelogscheck-skipped = ChangelogsRequired conditions to stay in the queue
#review-threads-unresolved = 0[🛡 GitHub branch protection]github-review-approved[🛡 GitHub branch protection]check-success = Doctest Cabalcheck-neutral = Doctest Cabalcheck-skipped = Doctest Cabalcheck-success = Meta checkscheck-neutral = Meta checkscheck-skipped = Meta checkscheck-success = docs/readthedocs.org:cabalcheck-neutral = docs/readthedocs.org:cabalcheck-skipped = docs/readthedocs.org:cabalcheck-success = Validate post jobcheck-neutral = Validate post jobcheck-skipped = Validate post jobcheck-success = fourmolucheck-neutral = fourmolucheck-skipped = fourmolucheck-success = hlintcheck-neutral = hlintcheck-skipped = hlintcheck-success = Bootstrap post jobcheck-neutral = Bootstrap post jobcheck-skipped = Bootstrap post jobcheck-success = whitespacecheck-neutral = whitespacecheck-skipped = whitespacecheck-success = Check sdist post jobcheck-neutral = Check sdist post jobcheck-skipped = Check sdist post jobcheck-success = Changelogscheck-neutral = Changelogscheck-skipped = Changelogs