Repository navigation
Nightly-lane test fixes: strudel worker-clock waits, a refused -exe fails, ownership_semantics passes the collect proof - #4161
Conversation
|
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The changes are test-only, low-risk, and internally consistent, but they alter concurrency/timing synchronization whose flakiness-elimination can only be confirmed by the author's runtime validation under sanitizers, warranting human sign-off.
Review effort: Balanced
Findings: None
What changed in this PR
This PR makes the strudel device-audio tests robust to slow execution under the tsan-tests nightly lane, where a sanitizer build on a contended runner renders audio far slower than real time. Previously the tests budgeted wall-clock time, so a slow-but-working worker failed as if stuck. The tests now synchronize on the worker's own playback clock (seconds of audio rendered) and fail only when that clock genuinely stalls (no advance for 30 s of real time). It also improves a test_vowel failure message to report the measured peak/energy.
Changes:
- Adds a shared
_strudel_device_common.dasmodule (worker_seconds,wait_worker_untilwith a stall-timer that resets on each clock advance) and reuses it across the worker tests. - Refactors
test_worker_heapto poll on worker time (dropping the 90 s wall cap and reporting a stalled stretch) and relaxestest_sound_status_seqboxto acceptplayingas well asstarting, with a 10 s tone so a slow reader can't see it end. - Adds energy tracking and a descriptive failure message to
test_vowel.
| File | Description |
|---|---|
tests/strudel_device/_strudel_device_common.das |
New shared helper: worker clock accessor and stall-aware wait_worker_until (2-arg + convenience 1-arg overloads). |
tests/strudel_device/test_worker_heap.das |
Drops local clock/wait helpers and the wall cap; sample_stretch now polls via a block, adds a finished flag surfaced in the assertion message. |
tests/strudel_device/test_worker_no_gc.das |
Replaces fixed sleeps with wait_worker_until; uses the shared module. |
tests/strudel_device/test_sound_status_seqbox.das |
Accepts starting or playing after registration; plays a 10 s tone; updated test description. |
tests/strudel/test_vowel.das |
Accumulates energy and reports peak/energy in the tick-test failure message. |
Verification highlights: ref_time_ticks→int64, get_time_usec→int (so WORKER_STALL_MS * 1000 = 30,000,000 stays in int range); success(tb; a; msg="") supports the added messages; the continue if→return if rewrite is correct because a $-block return is block-scoped (confirmed against daslib/lint.das:1390); _-prefixed helpers are excluded from test discovery (dastest/fs.das:106); and no stale references to the removed WALL_CAP_MS/old signatures remain.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
…fused -exe fails; ownership_semantics passes the collect proof
The tsan-tests nightly lane failed test_worker_heap ("7 then 0 distinct numbers" at the 90 s wall
cap) and test_worker_no_gc (no heap published 2.5 s in). Under tsan on a contended 4-core runner the
worker renders audio at a fraction of real time, so a wall-clock budget fails a slow worker as if it
were stuck. tests/strudel_device/_strudel_device_common.das waits on the worker's own clock - seconds
of audio rendered - and fails only when it stands still for 30 s of real time: test_worker_heap drops
its total wall cap and reports a stretch the stall cut short, and test_worker_no_gc waits on the
worker's clock instead of fixed sleeps. The module name is unique: a second `_common` loses to
tests/linq's shared one when a single process compiles the whole suite.
test_sound_status_seqbox read `playing` right after set_status_update under the same load - the
audio thread picked the sound up before the read. The box still must not read empty; `starting` or
`playing` both pass, and the tone runs 10 s so a descheduled reader cannot see it finish.
A refused -exe build now fails the process: the exe branch of llvm_jit_run logged the refusal
and returned success, so extended_checks' example runner compiled `ownership_semantics.das`
"clean" and then found no .exe to run. The example itself trips the collect-carrier proof -
`main` held a Terminal across its heap_collect - so the collect now runs in a `main` with no
locals and the checks move to their own function. The llvm_exe_thread_collect fixture's thread
and job lambdas collect on stacks the host starts empty; they and `main` carry
[unsafe_heap_collect], as the proof's escape for that case intends.
test_vowel_filter_tick_produces_output failed once under fastmath LLVM-AOT and does not reproduce;
its failure message now carries the peak and the energy, where a NaN shows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1356a85 to
3d87033
Compare
borisbat
changed the title
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It alters the LLVM JIT executable-generation driver's failure behavior and the GC collect-carrier proof semantics — subtle codegen/runtime areas whose end-to-end correctness was validated only in the author's local WSL build and warrants human sign-off.
Review effort: Balanced
Findings: None
Why. The tsan-tests nightly lane fails the strudel worker tests on a busy runner: under tsan on four shared cores the worker renders audio far slower than real time, and the tests budget wall-clock time, so a slow worker fails as if it were stuck. Separately,
extended_checksfailsrun_example_ownership_semantics: a refused-exebuild exited 0 without an executable.What changes.
tests/strudel_device/_strudel_device_common.daswaits on the worker's own clock (seconds of audio rendered) and fails only when that clock stands still for 30 s of real time.test_worker_heapdrops its 90 s total wall cap and reports a stretch a stall cut short;test_worker_no_gcwaits on the worker's clock instead of fixed sleeps.test_sound_status_seqboxacceptsplayingas well asstartingright afterset_status_update, and plays a 10 s tone so a slow reader cannot see it end.test_vowel's tick test failure message reports the peak and the energy.-exebuild fails the process (LLVM EXE: no executable written for ...), like a refused-lib, on both the single-module and the--jit-split-modulespath.ownership_semantics.dasruns its collect in amainwith no locals, so the collect-carrier proof clears it; thellvm_exe_thread_collectfixture's thread and job lambdas carry[unsafe_heap_collect].Observable behavior.
daslang -exeon a program the collect proof refuses: exit 0, no executable -> exit 1 with the reasonextended_checksrun_example_ownership_semantics: "no such file" -> the example builds and runstest_voweltick failure:expected success, got failure-> the peak and energy it measuredWhere to look.
wait_worker_untilin_strudel_device_common.das, and the relaxed state check intest_sound_status_seqbox.das.#nightly
Validation, claims, ledger
Validation
tests/strudel_devicepinned to 4 cores beside 6 busy loops: before, 1 of 3 runs red with the nightly's signature (20 then 0 distinct numbersat 91 s) plus the seqboxplayingrace; after, 5 of 5 green. Unloaded: 30/30.tests/linqthentests/strudel_device(therelease.ymlshape): 2104/2104; the same with the helper named_commonfails to compile against linq's module.test_vowel: interpreter,-jit, andtest_aot(--target test_aot, dasAudio on, WSL) pass; the fastmath LLVM-AOT lane rebuilt verbatim ran 11489/11489 and the tick test 0/300 red - the one CI failure did not reproduce.-exefix (WSL, current master, LLVM on): the old example now exits 1 with the new message; the restructured one builds, and its exe and the interpreter run clean;run_examplesall pass;llvm_exe_thread_collect2/2 (refused silently on master);tests/jit_tests400/403 (3 skipped); a refused--jit-split-modulesbuild exits 1. dasLLVM's module suite (dastest -jit --test modules/dasLLVM/tests, same fastmath build): master 105/130 with 24 failing, this branch 106/130 with 23 - the one difference isllvm_exe_thread_collect; the rest fail identically on both (llvm_jit_debug_info8,llvm_jit_link6,llvm_vector_math5,llvm_jit_global_lookup2,llvm_tune_profiles1,llvm_exe_split_lto1).Claims - stated, not tested
playingfrom a seed ofstartinginset_status_update; an empty box still fails.Not done
test_vowelred under fastmath LLVM-AOT: LLVM-AOT targets the host CPU and the runners vary; not reproducible on a Zen 2 box without AVX-512. Ledgered.llvm_exe_split_ltofails 1/7 on master: it looks for aLOG_INFOannounce line that the defaultDAS_LOG_LEVELdrops. It andllvm_exe_thread_collectrun in no CI lane, which is how both rotted unseen. Ledgered.make-pr --no-preflightprints "all mechanical gates green" without runningreview-mdorast-verify: ledgered as a tool fix.