Repository navigation
verify: pin the serve host thread the session loop already pins - #501
Closed
yannickloth wants to merge 1 commit into
Closed
yannickloth wants to merge 1 commit into
yannickloth wants to merge 1 commit into
Conversation
Verifier::run spins on the doorbell and then joins the CPU expert pool's drain, but unlike SessionLoopScratch it never pinned its host thread, so the host was free to time-share a pool worker's core. Pin it once per verifier to the core ExpertPool reserves for the host (host_pin_core: the first physical core physical_cores(true) drops, never a worker's), and restore the caller's affinity in the destructor when the same thread destroys the verifier. A layer split does not pin (host_pin_core returns -1): all stages share one reserved core and a pin would serialize them. STRATA_NO_VERIFY_PIN is the same-binary A/B off arm; nothing in the token loop changes and no numerics change. Measured on the RTX A3000 + i7-12850HX host: there is no collision to fix in production 0.1.35, because SessionLoopScratch::init runs unconditionally in main() before the serve verifier block and already pins the host to core 0 (workers on 2,4,6,8,10,12,14,16). Same-binary A/B, 6 interleaved runs per arm, fixed prompt: off median 25.75 / mean 25.88 tok/s, on median 24.45 / mean 25.82 tok/s - mean -0.3%, no >3% win. Fixed-prompt output bit-identical. This is a robustness fix: the reservation is now enforced by the verifier itself rather than by the session scratch that happens to run first. Tests: verify_host_pin_test, pool_test, pool_stress, expert_parity, file_expert_source_test, expert_layout_test, expert_cache_per_layer_test and the CPU parity set pass (expert_multi_test still refuses on a no-AVX-512 CPU). Multi-GPU layer splits can only be argued here: this is a single-GPU host. Refs Niko1221#494, Niko1221#489, Niko1221#485.
Contributor
Author
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
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.
Pin the serve/spec host thread the way the session loop already pins its host.
What this targets
SessionLoopScratch::initpins the host thread to the coreExpertPoolreserves(
physical_cores(false)[0]; workers come fromphysical_cores(true)), because the hostjoins the pool drain with the default
host_works(generate.cpp:2637) - the startuplog's "8 expert-pool workers + the host thread".
Verifier::run(verify.cpp) spins onthe doorbell and then calls the same pool, but never pinned, so its host was free to land
on a worker's core or SMT sibling. See #494: the CPU pool is 53% of a fresh round / 26%
warm, and the host is one of the ~9 draining threads.
Measured first (#494): no collision on this host
On production 0.1.35 there is no collision to fix:
SessionLoopScratch::initis calledunconditionally in
main()(generate.cpp:3607) before the serve/verifier block(
:4200), so the serve host is already on the reserved core. During an active servedecode the only busy non-worker thread is the engine main thread, with
Cpus_allowed_list: 0, and the 8 workers on2,4,6,8,10,12,14,16; 120 x 50 ms samplesgive exactly nine busy threads, the host on core 0 in every sample.
The fix
Verifiernow carries the same pin/restore asSessionLoopScratch:host_pin_core(split): the first physical corephysical_cores(true)drops (never aworker's), or -1 under a layer split / when no such core exists.
maybe_pin_host()once at the top ofrun(), before the spin:STRATA_NO_VERIFY_PINis the off arm; refuses when the calling thread's affinity does not include the core;
pins once and records the previous mask and thread id.
serialize.
No numerics change, no token-loop change, no pool worker-count or affinity change.
A/B (same binary,
STRATA_NO_VERIFY_PIN)Local
build/strata,bench/e2e.sh, fixed prompt,max_tokens 200, interleaved, 6runs per arm, alternating order:
Mean delta -0.3%, median -5.1%: not the >3% win
tools/calibrate.pyrequires(the host is noisy, +/-20%). This is a robustness fix, ~0 perf: it makes the
reservation local to the verifier instead of depending on the session scratch that
happens to run first.
Evidence
and the workers on 2..16;
/proc/<pid>/task/*/statusconfirms.verify_host_pin_test,pool_test,pool_stress,expert_parity,file_expert_source_test,expert_layout_test,expert_cache_per_layer_testand 10CPU/GPU parity tests pass.
expert_multi_testfails its pre-existing no-AVX-512refusal (this host has AVX2 + AVX-VNNI).
verify_host_pin_testasserts the chosen core is not a worker core, and that a layersplit returns -1.
Caveat
This is a single-GPU host, so the layer-split guard is verified by inspection and by the
unit test, not by running a multi-GPU split.
maybe_pin_hostskips whenevernext_ != nullptr || lb_ > 0 || le_ < g.n_layers, i.e. every stage.Refs #494, #489, #485.