Replies: 4 comments 11 replies
|
@hunhoffe I suppose you might want to have a look at this, since CI looks being painful a little |
|
@atassis One piece for why the compilation was originally included, is that some chess builds can be very memory intensive too, so we were oomkilling the CI runners if we didn't limit concurrency specifically for chess quite a bit. We might need to be careful to keep the chess tests serial. @thomthehound Tagging you to make sure you see the windows notes! |
|
@hunhoffe thanks, that memory point is the key constraint and I don't want to fight it. Agreed the Chess compiles have to stay bounded. I think the split is compatible with keeping them bounded, because it separates two limits that are currently fused. The capacity-1 Where that turns concrete: I can't run Chess locally, so I built a temporary, self-guarding validation job to gather the real numbers on a Chess runner. It runs whole vs split and asserts they match, plus two deliberate-failure controls (execute-only on a wiped root must fail every test; a mutated kernel must fail in both modes), encoded as CI assertions so a silent regression fails the job rather than passing quietly. Running it on a Chess-capable runner would give the split==whole correctness on Chess, the timing, and the compile memory footprint, so the right compile concurrency cap is a measured number rather than a guess. Would you be open to running that, or would you prefer I wire it a particular way? Draft RFC is #3403; happy to add a memory-sized compile group to it so the Chess-serial property is explicit. |
|
@atassis I've been thinking through #3404, and I'm still a little bit conflicted. The two things I'm concerned about are:
Part of me thinks we should just add CI machines with more granular jobs to make it go faster without adding more infrastructure complexity. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
I spent some time profiling why the
buildAndTestRyzenAIjobs are the long pole, and I think the framingmatters, so I wanted to put the numbers somewhere trackable rather than in a drive-by PR. Opening this per
the tracking discussion that was suggested; happy to fold it into #3198.
The finding. The capacity-1
npu-xrtlit group serializes each device test end to end -- hostaiecc/xclbin generation and the device invocation, not just the device milliseconds. On this hardwarethe generation dominates: for the simple Chess-backed tests the compile is ~95-100% of the test's
wall-clock (e.g. a 26 s test whose compile-only twin is 26 s), and across the serialized lane the honest
planning interval is ~85-95% compile. So the lane is compile-bound, and compilation touches no device.
The mechanism, framed as de-loading. I know coordination has been tried and deliberately backed out
here (the device lock experimented under #2914 and removed in #2992 with "the problem is not one of
concurrency", and the merge-queue concurrency reverted for job ejections). I am not proposing to add
coordination. The opposite: an opt-in, per-test compile/execute split that moves the compile half OUT
of the capacity-1 group so it runs in parallel with everything else, and leaves ONLY the device execution
in the serialized lane that #2737 added to protect the shared XRT hw_context pool. Strictly less on the
serial lane, no new lock, no cross-job handoff.
Concretely it is a marker on each RUN line -- a build-phase prefix and a run-phase prefix that expand to
nothing in a normal run and to a shell no-op in the phase that should skip. A test opts in by carrying the
markers; everything else is unchanged and stays serialized. Redirect/pipe/FileCheck/JIT tests that have no
clean compile/execute boundary are simply not converted.
Evidence (on a Krackan AIE2P box, single-tenant). I built a matched-commit toolchain and ran the
converted
npu-xrttests three ways -- whole test, compile phase, execute phase -- and compared verdicts:verdict as the whole test, 0 mismatches on a clean run. The one transient failure I saw was an
equal-value mismatch (
5 != 5), i.e. the known host/device read race the workflow already retries on,not a split artifact (it passes in isolation in both modes).
is a lower bound on the win -- these are fast Peano compiles (~1-3 s); the real
check-aiesink is theChess compiles at 26-77 s each, which the split relocates identically, so the compile-bound jobs benefit
far more.
Two smaller, orthogonal levers I also measured, for completeness: the on-device workflow has no
paths:filter (themlirDistroone does), and PR-synchronize churn cancels ~8.8% of device-job time.Path filters are a safe, zero-coverage-loss config change; dropping
pull_requestto merge_group-only is areal spend/pre-merge-signal trade I would leave to your judgment, since the suite does catch real
pre-merge failures.
One further idea, raw and unmeasured -- floating it for the tracking record. If the split lands, the same
compile-vs-execute seam generalizes across OS. The device payload (
.xclbin+npu-insts) is a siliconbinary, so in principle it could be built ONCE on a Linux host (reusing that one build cache) and executed on
both the Linux and Windows NPU runners, with the Windows host harness cross-compiled on Linux (clang
targeting Windows -- cross-compilation, not emulation). The Windows leg would then skip recompilation and
share the Linux cache. The load-bearing question is one you would know far better than I do: does the
.xclbincarry any host-OS-specific bits, or is the AIE2P payload OS-independent? If it is independent,"compile once on Linux, execute on each OS's runner" removes a whole duplicate compile. Not proposing it --
just recording it in case it is useful later.
The split is already up as a small, individually-reviewable draft RFC (#3403) -- plumbing plus a handful of
converted tests -- if it helps to look at something concrete. Separately, #3402 narrows the on-device retry
so a transient failure re-runs only the failed tests instead of the whole job (the mechanism behind the
occasional ~1-hour outliers). Mostly, though, I wanted the compile-bound attribution on the record so the
optimization aims at the right half.
All reactions