fetch-pack: derive tunable worker defaults for URI packs - #75
Open
uoleg-oai wants to merge 37 commits into
Open
Conversation
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.
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>
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.
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.
In the following commit(s), some tests will need to distinguish between `REF_DELTA`s and `OFS_DELTA`s to exercise a new '--no-ref-delta' option for 'pack-objects'. Existing tools report delta relationships, but not how their bases are represented in the pack. Teach 'test-tool pack-deltas' a '--list-deltas' mode. For each delta entry, print the object ID, its REF_DELTA or OFS_DELTA type, and the base object ID or pack offset, respectively. This lets tests inspect pack headers without open-coding a parser. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Some consumers of 'pack-objects' may wish to avoid packs which contain `REF_DELTA` entries. For instance, a 'receive-pack' implementation which retains the resulting pack without building an index of object IDs may prefer every delta base to be discoverable from an earlier entry in the same pack. Teach 'pack-objects' a new `--no-ref-delta` option to avoid writing `REF_DELTA` entries, without changing whether `OFS_DELTA` is allowed. When used without `--delta-base-offset`, no delta representation remains, so avoid delta search entirely. Otherwise, allow new deltas whose bases appear earlier in the same pack. For now, disable delta- and bitmap-reuse under `--no-ref-delta`, since either may copy an existing `REF_DELTA` entry. This is overly pessimistic, but simplifies the changes in this commit. The next commit re-enables reuse in the cases which do not require `REF_DELTA`. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
The previous commit disables delta- and bitmap-reuse entirely whenever pack-objects is given '--no-ref-delta' for the sake of simplicity. This is overly pessimistic. When '--delta-base-offset' is also given, delta reuse can remain enabled. A reused delta whose base is written earlier in the output can be encoded as an `OFS_DELTA`, even when its source copy was encoded as a `REF_DELTA`. Preferred bases and external thin-pack bases are different: neither appears in the output, so deltas against either still require encoding the object as a `REF_DELTA`, and thus cannot be reused. Without '--delta-base-offset', delta reuse remains disabled, since no delta representation remains. Bitmap reuse follows a different path, since selected entries may be copied without passing through the code which chooses a delta representation. When given '--no-ref-delta', we must inspect candidate objects individually, and leave `REF_DELTA` entries to the normal object path outside of pack-reuse. We must likewise avoid the special-case for reusing either the single or preferred pack corresponding to the bitmap by whole `eword_t`'s at a time. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Add a 'no-ref-delta' receive-pack capability and teach send-pack to pass '--no-ref-delta' to 'pack-objects' when the server advertises it. Keep this separate from 'ofs-delta' so that a server may request that `send-pack` omit `REF_DELTA` without also accepting `OFS_DELTA`. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
A client can rely on a server no-ref-delta promise only if it checks every received pack. Add --no-ref-delta to reject a REF_DELTA while parsing the pack. OFS_DELTA remains accepted. By definition, an OFS_DELTA refers to an earlier entry in the same pack, independently of this option. Test file and stdin input containing REF_DELTA. Retain coverage that accepts OFS_DELTA. Signed-off-by: Friel <friel@openai.com>
The existing no-ref-delta capability lets a receive-pack server ask send-pack not to emit REF_DELTA entries. Protocol v2 fetch needs the same guarantee in the other direction before a client can treat packfile URI responses as independent packs. Add uploadpack.allowNoRefDelta and advertise no-ref-delta as a fetch feature when it is enabled. When a client requests it, pass --no-ref-delta to the inline pack-objects process. The same promise covers every configured packfile URI; upload-pack cannot inspect those packs, so the administrator must create or verify them accordingly. The promise remains independent of ofs-delta. Test advertisement, rejection of an unadvertised request, and the pack-objects argument. Signed-off-by: Friel <friel@openai.com>
Packfile URI responses are indexed in advertised order because a later URI pack can contain a REF_DELTA whose base is installed by an earlier response pack. That ordering prevents independent URI packs from using more than one indexer. When upload-pack advertises no-ref-delta and more than one URI job is configured, request it with packfile-uris. Pass --no-ref-delta to the inline indexer and every URI indexer, then run URI packs concurrently. Keep each URI indexer at one thread so the configured job count also bounds indexer threads. Precreate each expected keep path before starting its http-fetch child, record newly-created paths in the existing pack-lock list, and omit the child's --keep argument. Existing fetch cleanup then owns keeps created by this process while pre-existing keeps remain untouched. Mark both helper process layers for exit cleanup. Use a small poll loop to refill a finished slot until all URI packs are indexed. One job, one URI, or callers without pack-lock ownership retain the serial path. A promised URI pack that contains a REF_DELTA fails closed in index-pack. Test the negotiated request, inline verifier, and three URI indexers. Signed-off-by: Friel <friel@openai.com>
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.
Integrate the current tb/codex/automation topic into the internally distributed codex branch. Codex-Integration: tb/codex/automation@9b7652c87e369dafb6a50e97d24670ccd512f1bc
Integrate the current tb/codex/geometric-maintenance-promisor topic into the internally distributed codex branch. Codex-Integration: tb/codex/geometric-maintenance-promisor@bccc1fd882f27143f7331192a75c6281ba76e2f7
Integrate the current tb/codex/release topic into the internally distributed codex branch. Codex-Integration: tb/codex/release@2bb6c1c18aa3170efff362eb3fb8f096134955f7
Integrate the current dr/codex/dugite topic into the internally distributed codex branch. Codex-Integration: dr/codex/dugite@5b75aebc9071c08a87ddf7bf00b173e05a5bfc96
Integrate the current tb/codex/lto-pgo topic into the internally distributed codex branch. Codex-Integration: tb/codex/lto-pgo@67ba20645b1792f43c8c8ea69c716687bda87ec3
Integrate the current tb/codex/packfile-uri-concurrency topic into the internally distributed codex branch. Codex-Integration: tb/codex/packfile-uri-concurrency@2c3adbb2c475981e340c79fdc5e7f4f9b5d9054e
Integrate the current af/codex/pack-bytes topic into the internally distributed codex branch. Codex-Integration: af/codex/pack-bytes@a6bb1bb89133e5eccf6e3ca3aaa58f560eb3744c
Integrate the current tb/codex/parallel-packfile-uris topic into the internally distributed codex branch. Codex-Integration: tb/codex/parallel-packfile-uris@6f0edce407d5e8c1d212514281f49c9bc433ee3c
Packfile URI fetches default to one job, and parallel URI indexers force one thread even when pack.threads is configured. Use online CPUs to choose defaults while retaining the existing configuration controls. Default fetch.packfileUriJobs to the online CPU count capped at eight. Divide CPUs among the actual parallel jobs for each indexer, with a one-thread minimum and four-thread automatic maximum. Explicit positive pack.threads overrides that budget; explicit URI job values remain uncapped and values at or below one retain serial processing. Preserve no-ref-delta negotiation and verification, serial fallback, pack ownership and cancellation. Document that increasing both controls can increase CPU and memory use. Verification: the new default-concurrency regression fails on the old implementation and passes with the change. The developer build and all 102 t5702 protocol tests pass, including real HTTP coverage, explicit thread values, job overrides and no-ref-delta validation. All 70 applicable t5300 index-pack tests pass; two !PTHREADS cases do not apply to this threaded build. Chainlint, greplint and documentation style checks pass. Independent review found no introduced P0/P1/P2 defects.
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
ttaylorr-oai
force-pushed
the
codex
branch
3 times, most recently
from
September 9, 2026 00:14
809b7da to
777246e
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.
Packfile URI fetches default to one job, and parallel URI indexers force
one thread even when pack.threads is configured. Use online CPUs to
choose defaults while retaining the existing configuration controls.
Default fetch.packfileUriJobs to the online CPU count capped at eight.
Divide CPUs among the actual parallel jobs for each indexer, with a
one-thread minimum and four-thread automatic maximum. Explicit positive
pack.threads overrides that budget; explicit URI job values remain
uncapped and values at or below one retain serial processing.
Preserve no-ref-delta negotiation and verification, serial fallback,
pack ownership and cancellation. Document that increasing both controls
can increase CPU and memory use.
Verification: the new default-concurrency regression fails on the old
implementation and passes with the change. The developer build and all
102 t5702 protocol tests pass, including real HTTP coverage, explicit
thread values, job overrides and no-ref-delta validation. All 70
applicable t5300 index-pack tests pass; two !PTHREADS cases do not apply
to this threaded build. Chainlint, greplint and documentation style
checks pass. Independent review found no introduced P0/P1/P2 defects.
-- [ uoleg-codex (source thread) ]