perf(core): reuse a persistent weight-fill pool and tune staging geometry#763
perf(core): reuse a persistent weight-fill pool and tune staging geometry#763FeathBow wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a469588457
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
a469588 to
58152a0
Compare
|
Re-checking the measurements surfaced a second problem, in the GH200 row of the body. Those figures difference medians of two independent arms across repetitions; only same-repetition pairs are comparable here, because the phases around the load drift with machine state. Read as pairs, that run yields one usable repetition — the baseline arm of the first ran against a cold page cache, since the job had no untimed warm-up, and the second was hit by unrelated load on the node. The surviving pair is 4729 → 4315 ms in phase and 9608 → 9150 ms to ready. The sm_89 row stands: five clean pairs, load phase -8.8% to -11.0%, ready flat. The in-phase effect is real and the two hosts agree on its size; the GH200 ready figure is one repetition, a direction rather than a claim. The twelve-versus-eight comparison is unpaired too, though its ranges do not overlap. A re-run with a warm-up and five repetitions is queued to replace the GH200 row. |
|
The persistent-pool follow-up is folded into this PR, because the grid below shows the geometry change alone is dominated by the pool alone. Two GH200s, Qwen3-32B, tensor parallel size two, 144 cores. Ten arms, five repetitions, arms interleaved inside each repetition, one untimed warm-up discarded. CPU is sampled every 100 ms and read at the weights-ready mark, so it covers the fill rather than the whole startup. Spread is (max-min)/median of the phase across repetitions.
Row one is main; row eight is this PR. The constants alone are not worth shipping. Spawning at 8 workers and 32 MiB, which is what this PR contained before, reaches 8272 ms to ready at 25.01 s of fill CPU; pooling at main's own 4 and 64 MiB reaches 8262 ms at 20.35 s — better on both axes with no constant touched. Per second of startup saved that arm costs 11.9 CPU-seconds, against 4.3 to 4.6 for every arm that includes the pool. CPU is bought by the worker count, not the chunk size. At fixed chunk, four workers to eight costs 14.3% and 17.5% more fill CPU on the pooled arms; at fixed workers, 64 MiB to 32 MiB costs 1.1% and 4.0%. The two were changed together in one step before, which is why the earlier numbers attributed the cost to the wrong variable. The price of this PR is 6.68 additional CPU-seconds once per process start for 1547 ms off HTTP-ready, on a host where the fill already occupies about four cores and now occupies about nine. Cheaper points exist on the frontier if that is the wrong trade: pool at 8 workers and 64 MiB is 7651 ms for 5.76 CPU-seconds and leaves pinned memory at 128 MiB. On a single sm_89 GPU with Qwen3-4B the same comparison is flat to ready across five paired repetitions. That host reaches its H2D roofline of 23.3 GiB/s while four fill threads already sustain 24.6 GiB/s, so a fill-side saving has nowhere to surface. Validation. Open question. |
|
Rebased onto main after #752 merged and re-measured there, so the figures in my earlier comment. What ranOne sm_89 GPU: Two GH200s: Correctness
Measurements
Milliseconds for ready and load, seconds for cpu. Repetition 5 sits apart from the other four in both the branch arm (load 1720 against about 850) and main (ready 8219 against about 7600). Same-repetition pairs, main to this branch: ready -14.4%, -7.3%, -4.4%, -8.0%, +2.2%; load -23.5%, -24.5%, -25.3%, -18.4%, +45.1%. Medians across arms: ready -8.7%, load -24.3%, cpu +34.3%. On the pre-#752 base the same change measured -17.4% to ready, and the CPU delta was of the same size, which puts the cost at 7.7 CPU-seconds per second of startup saved here against 4.3 there. Correction to the grid comment aboveThe Recomputed from CPU read at ready, which is a direct
The statements that column supported come out the same: four workers to eight costs 12.6% and 14.8% more CPU on the pooled arms while 64 MiB to 32 MiB costs 0.3% and 2.2%; spawning at 8 and 32 MiB reaches 8272 ms at 28.84 s against pooling at main's constants reaching 8262 ms at 24.25 s; per second of startup saved that arm costs 12.1 CPU-seconds against 4.3 to 4.9 for the pooled arms. A constant common to every arm cancels in the differences, which is why the percentages shift and the orderings do not. What did not runNo TP4 or TP8 of anything — that is what the No other model line was exercised. That is bounded rather than assumed: Two figures are not comparable across the two campaigns even though both are labelled a load phase: the earlier one came from a tensor-parallel timeline marker covering both ranks and a barrier, this one from the model's own per-rank line. Open question
|
fb18907 to
7e69c6f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e69c6f05e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
7e69c6f to
0552a42
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0552a424b6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Feathbow <feathbow@gmail.com>
0552a42 to
fee39e9
Compare
Description
The pinned staging path shipped with 64 MiB buffers filled by four threads. Both numbers were picked to amortize the per-chunk event sync rather than measured against the fill rate itself, and neither had been swept as a pair. This changes only those two constants; no logic moves.
Measured on two hosts, warm page cache, arms interleaved inside each repetition so a drift in machine state cannot masquerade as an arm effect:
Medians of three repetitions on the first host, five on the second.
The two hosts disagree at HTTP-ready for a reason worth stating: the sm_89 host reaches its H2D roofline of 23.3 GiB/s while four fill threads already sustain 24.6 GiB/s, so the pipeline there is transfer-bound and an in-phase saving cannot surface end to end. The GH200 host is not, so it does. Both hosts agree the load phase itself gets faster, and neither regresses at ready — the sm_89 spread across repetitions of a single arm is 17 ms, wider than the 10 ms difference between arms.
Twelve threads was measured on the GH200 host and is slower than eight (4406 ms against 3937 ms) while costing a third more CPU, so the thread count stays a fixed width rather than a fraction of the core count. It is still capped by
available_parallelismso a small host does not oversubscribe, and logs the effective width when that cap engages.Two consequences beyond throughput. Pinned memory halves to 64 MiB across both buffers. And
upload_colsrejects a column shard whose row width exceeds one staging buffer, so halving the buffer halves that ceiling — from 33.5 M to 16.7 M columns per shard, which no published checkpoint approaches.Validation on the sm_89 host:
openinfer-coreunit tests pass, and a greedy completion is byte-identical before and after the change, with a different prompt producing different output as a control.The TP=2 pass of
hf_golden_gatehas not been run on this branch; it needs two visible CUDA devices and self-skips below that. It is the gate for the column-gathering path, whose chunk count this change alters.Type of Change
Checklist
docs/conventions/coding-style.md).CLAUDE.md).