index-pack: hash full blobs in a bounded worker pool - #67
Draft
ttaylorr-oai wants to merge 35 commits into
Draft
Conversation
Topic pull requests merge into codex before the controller rebuilds its recorded output. Treating every codex push as a release would run the full build matrix and publish artifacts for an intermediate tree. Compare each pushed commit with the output recorded in meta:codex.config before starting version detection. Skip the build and release jobs unless the controller published that exact commit; a later pending merge must not suppress its release.
Commit 9bc1518 (builtin/maintenance: introduce "geometric-repack" task, 2025-10-24) added a new maintenance task to perform either a geometric repack, or an all-into-one repack if the geometric repack would itself produce a single pack. Some time later, commit dcc9c7e (builtin/repack: handle promisor packs with geometric repacking, 2026-01-05) taught the geometric repacking machinery to separate promisor packs from ordinary ones, but did not update the maintenance task accordingly. As a consequence, the geometric-repack maintenance task only considers the non-promisor pack progression. It falls back to all-into-one whenever a geometric repack would roll up all non-promisor packs into a single pack, even if the promisor progression would keep a large pack and roll up only smaller ones. Check both progressions before choosing the repack mode. If either leaves a pack above its split, geometric repack still avoids rewriting that pack, whereas the all-into-one fallback would rewrite it. Use the fallback only when neither progression leaves a pack behind. That preserves the reason for the fallback: let the all-into-one repack handle unreachable objects when it is not rewriting more packs than the geometric repack. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Commit 9bc1518 (builtin/maintenance: introduce "geometric-repack" task, 2025-10-24) added an auto condition for the geometric-repack task. It runs the task when ordinary packs need to be combined or when the number of loose objects crosses the configured threshold. Later on in commit dcc9c7e (builtin/repack: handle promisor packs with geometric repacking, 2026-01-05), the geometric repack machinery started handling promisor packs separately, but did not correspondingly update the auto condition. As a result, a repository can have promisor packs ready to combine while its non-promisor packs and loose object count require no work. In that case, `--auto` skips the task even though a geometric repack would combine at least two promisor packs. Check `geometry.promisor_split` alongside `geometry.split`. There is some fallout in t5331: the new condition makes a filtered clone eligible for auto-maintenance before the test inspects its promisor packs. Disable auto-maintenance in that fixture so it continues to test `--stdin-packs`, not the maintenance task. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
The release workflow currently listens only to codex and checks every push against codex.output-tip. The controller records and atomically publishes codex-unstable.output-tip separately, so preview output pushes need an independent gate. Listen to both generated branches, skip deletion events, and select the recorded output from the exact pushed ref. Unknown refs and missing state fail closed. The existing version, build, and release chain still runs only when the selected output matches GITHUB_SHA.
Codex and codex-unstable releases currently share the same prerelease shape. Their target SHA differs, but the GitHub release object does not record which output ref triggered it. Consumers must therefore join release data with Actions runs to distinguish the lanes. Carry github.ref into the publish step and put source_ref and source_sha at the start of the release notes. This leaves tag and asset names unchanged while giving release-API consumers a machine-readable lane marker.
The default-branch trampoline must not start from pull_request_review: that event loads workflow code from the PR merge ref. Replace it with a trusted default-branch scanner. On a five-minute schedule, or an explicit scan dispatch, it pins meta, checks it out read-only, preflights approved candidates with propose-plan --no-push, and calls the App-backed producer only for a valid exact-head review. Stale or policy-invalid approvals are skipped instead of blocking later topics. Plan admission stays pull_request_target with trusted meta code; remove and reorder remain explicit dispatch-only policy operations.
The release workflow cross-compiles Linux arm64 on an x64 runner and skips the smoke test for arm64 POSIX bundles. That prevents the workflow from executing the Linux artifact it just produced. Run Linux arm64 on GitHub's arm64 runner and install native development packages rather than configuring a foreign dpkg architecture. All matrix entries can then run the existing distribution smoke test.
Codex consumes Git release artifacts built with the Makefile's default -O2 flags. The release job compiles each artifact without link-time optimization. Add a release-only config.mak.openai and copy it into Git's ignored config.mak slot before building. Use thin LTO for Clang targets and automatic LTO for GCC targets, then check GIT-CFLAGS records the selected flag in every distribution job. Keeping the setting in config.mak.openai avoids carrying release-only policy in the upstream Makefile.
LTO can optimize across translation units, but the release job has no execution profile for the status, diff, clone, fetch, and repack paths Codex invokes frequently. Git's built-in profile target runs the 1,048-script test suite serially. That is too expensive for every release target and weights test-harness paths more heavily than the local workload. Extend config.mak.openai with GCC and LLVM profile modes. Gate GIT-CFLAGS on an instrumented build, run a short offline trainer, merge LLVM raw profiles when needed, and rebuild with profile-use flags. Each matrix entry runs on its target architecture, so it can execute the instrumented binary. Check that final GIT-CFLAGS includes a profile-use flag and increase the timeout for the second compilation pass. The focused trainer took about 30 seconds locally; the full macOS build/install validation completed with thin LTO and LLVM profile-use enabled.
The --packfile mode accepts one --index-pack-arg=<arg> option per argument passed to index-pack, but its documentation and option dependency errors still refer to the plural --index-pack-args form. Correct the spelling and describe the repeatable per-argument form. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
finish_http_pack_request() passes its staging-file descriptor to index-pack through child_process.in. start_command() takes ownership of a supplied descriptor and closes it, even when starting the child fails. Do not close the descriptor again after run_command() returns. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A resumed pack request may already have all bytes of the remote pack. A server can respond to the resulting Range request with HTTP 416 instead of returning an empty response. Accept that response in each pack-download caller and let index-pack validate the completed staging file. This can happen without concurrent downloads when a previous attempt completed the transfer but failed before indexing it. Add a regression test that seeds a complete partial pack and checks that http-fetch indexes it after the server returns HTTP 416. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pack requests stage downloads in a predictable partial-pack file so an interrupted transfer can be resumed. Both packfile URI and ordinary dumb HTTP requests use this staging path. Opening it in append mode forces each write to the current end of the file, so concurrent responses can append duplicate data and corrupt the pack. Open the partial pack read-write without O_APPEND and seek once to its current end. Each downloader then retains the offset matching the Range it requested. Because the staging key must uniquely identify immutable pack contents, overlapping responses write the same bytes at the same offsets instead of extending the file with duplicate data. Duplicate the staging descriptor for index-pack instead of reopening the path after closing the stream. Another downloader may unlink the staging path before indexing begins, but index-pack can still read the retained descriptor. Exercise resumed transfers and overlapping 200 and 206 responses, and clarify the staging-key documentation. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
On Windows, an open file must permit FILE_SHARE_DELETE before another process can unlink it. MinGW's non-append O_RDWR open enables that sharing mode only for an existing file; adding O_CREAT falls back to _wopen(), which cannot set it. First try opening the partial pack without O_CREAT. If it does not exist, create it exclusively, close that descriptor, and retry through the existing-file path. A racing creator retries after EEXIST. This ensures that every retained descriptor permits another downloader to unlink the staging path. Add an unlink-while-indexing test that does not require FIFOs and can therefore run on MinGW. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When index-pack finds an existing keep file it reports pack rather than keep. Accept either result from http-fetch, and only register a keep lockfile when this fetch created it. Read the pack/keep prefix and hash without consuming any following fsck output, validate the reported pack hash against the advertised hash, and exercise a packfile URI fetch with a pre-existing keep file. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Integrate the current tb/codex/automation topic into the internally distributed codex branch. Codex-Integration: tb/codex/automation@5cd3b3771c0aac541add6fc3a7a97bcfe1749cbb
Integrate the current tb/codex/geometric-maintenance-promisor topic into the internally distributed codex branch. Codex-Integration: tb/codex/geometric-maintenance-promisor@dc2fffc37cead551f8036c9ecab5e52a4cbee37b
Integrate the current tb/codex/release topic into the internally distributed codex branch. Codex-Integration: tb/codex/release@40589b5333835ecd5e1b6187cbcec45d8382303e
Integrate the current dr/codex/dugite topic into the internally distributed codex branch. Codex-Integration: dr/codex/dugite@6e6ec362f3c5e03f01bf1fd0fbdea5589cf4caec
Integrate the current tb/codex/lto-pgo topic into the internally distributed codex branch. Codex-Integration: tb/codex/lto-pgo@f0ee69fd806fa486eb3d00e799cfe564a499f8ab
Integrate the current tb/codex/packfile-uri-concurrency topic into the internally distributed codex branch. Codex-Integration: tb/codex/packfile-uri-concurrency@4cc8b3223b233aa3b795b1265e6e7cf3d63c7170
Integrate the current af/codex/pack-bytes topic into the internally distributed codex branch. Codex-Integration: af/codex/pack-bytes@9abdcdc650f32a4b03c31d0595e0b29892749574
The first pass through a pack inflates and hashes each object on the main thread. `pack.threads` applies later while resolving deltas, so SHA1DC work for full blobs remains serial even when CPUs are idle. In a 99 Hz profile of an 844,020,252-byte pack dominated by full blobs, SHA1DC accounted for 72.14% of sampled user CPU. Add an opt-in worker pool for complete heap-backed blobs. The producer continues parsing, inflating, computing CRCs, and writing the pack. The workers use the normal object hashing backend, including SHA1DC, while the main thread performs the existing ODB, collision, and content checks as deferred results are retired in queue order. Bound retained data by both bytes and job count. Strict, fsck, promisor, non-threaded, streamed, and otherwise ineligible objects retain the serial path. The default is disabled. Across three runs of the original prototype on that pack, two workers reduced median wall time from 36.076 to 19.227 seconds. CPU changed from 50.121 to 50.955 seconds and peak RSS from 413.7 to 409.8 MiB. The object/delta-heavy control did not show a wall-time improvement. The fixed-size queue used here has not been rebenchmarked. t5352 covers output equivalence, queue limits, serial fallbacks, collision and duplicate handling, REF/OFS ordering, corrupt input, and configuration validation. p5352 compares serial, one-worker, and two-worker indexing on a reproducible full-blob pack. Existing SHA-1 and SHA-256 index-pack tests also pass.
ttaylorr-oai
force-pushed
the
tb/codex/index-pack-perf-wip
branch
from
August 22, 2026 03:14
4bb2a85 to
633e355
Compare
ttaylorr-oai
added
kind:review-only
ttaylorr-oai
force-pushed
the
codex
branch
2 times, most recently
from
August 26, 2026 00:36
e6f5d83 to
47c847c
Compare
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
4 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.
No description provided.