Vm test cleanup - #4
Merged
Merged
Conversation
CLAUDE.md named the stack machine as the active MiniVM, which is exactly backwards: the register machine is the only supported target. It also left `bc_emitter` as the subject of the no-Zig-strings rule. Both now name the register emitter, and the note that `target: :bc` is the bytecode lowering mode -- shared by the register machine, not a stack-machine flag -- is written down so `bc_target?` is not mistaken for dead code during the stack removal. bc_lower_coverage only re-lowered the corpus with target: :bc and never emitted, while its comments explained themselves in terms of the incomplete `_bc_runner`. Drive RegisterBcEmitter over each lowered program so the emitter's own arms are covered too. Emission failures are rescued separately from lowering failures, so the existing accounting is unchanged: shard 0/40 reports 121 lowered / 27 raised both before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
Deletes bc_emitter.rb and _bc_runner.clear, and with them the stack disassembler, Bytecode struct, and StackTarget in vm_golden_harness.rb. MiniVM::Golden.targets is now register-only. bc_run.rb and run_tests.rb are kept, not deleted: both are the shared entry points the register machine runs through -- the golden harness shells out to `bc_run.rb <path> --run --vm=register`. Their stack branches are stripped and `--vm=` is accepted-and-ignored so existing callers keep working. The 19 CI-pending specs are gone with the reason for them. They were skipped because building vm.clear to a native binary timed out on GitHub runners; all 13 register-debugger specs and the 6 golden-harness run specs now execute unconditionally, and the golden-harness suite is 130 examples / 0 failures locally. No compiler changes. Both VMs lowered with `target: :bc`, so bc_target? and its 43 lowering sites belong to the register machine and stay. `--vm=register --min-pass=245` reports 237 passed / 39 pending / 0 failed both before and after this commit: that ratchet drifted while the Register-VM allowlist job sat disabled, and is not touched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
The gate asserted --min-pass=245, but the job has been `if: false` since the native-binary compile started timing out on hosted runners, so the ratchet went unenforced while the corpus moved under it. Actual state is 237 passed / 39 pending / 0 failed -- nothing fails, 8 entries went pending. Measured identically with and without the stack-machine removal, so this records where the corpus is rather than conceding ground: `--min-pass=237` now exits 0 with "baseline OK: 237 >= 237", and the ratchet can rise again from a number that is true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
Three problems, all costing cold compiles on every run. The six actions/cache steps store identical paths but keyed off four different prefixes -- clear-build-, clear-examples-coverage-, clear-fuzz-, clear-bench-leak-. restore-keys only matches its own prefix, so examples, benchmarks, and fuzz each maintained a private pool of the same content and none of them could warm from a sibling job. The extra per-job hashFiles inputs (examples/**/*.clear, tools/fuzz/**, benchmarks/**/*.clear) only narrowed the key; `./clear build` already keys each entry off its own source SHA, so a shared pool cannot serve a stale artifact -- it just carries entries a given job will not read. They now share the clear-build- prefix and its compiler/runtime inputs. examples/minivm/vm sits outside both cached paths, so the register VM binary was rebuilt on every fresh checkout even on a cache hit. Cached alongside them. bc_run.rb guards that build with an exclusive flock. Under prspec with 32 workers the first worker builds while the other 31 block, inside the parallel run. ruby-integration now warms it in a serial step first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
|
Bencher Report| Project | CLEAR |
| Branch | vm-test-cleanup |
| Testbed | ubuntu-latest |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | leak-build-ms | Measure (units) x 1e3 | leak-count | Measure (units) | leak-run-ms | Measure (units) |
|---|---|---|---|---|---|---|
| benchmarks/concurrent/01_socket_throughput/bench | 📈 view plot | 6.37 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 8.37 units |
| benchmarks/concurrent/06_dynamic_spawn/bench | 📈 view plot | 6.35 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 36.47 units |
| benchmarks/concurrent/11_parallel_aggregation/bench | 📈 view plot | 6.26 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 16.03 units |
| benchmarks/concurrent/18_atomic_counter/bench | 📈 view plot | 6.31 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 6.12 units |
| benchmarks/inter-clear/04_concurrent_mvcc_fat_struct/bench | 📈 view plot | 6.55 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 166.95 units |
| benchmarks/sequential/03_alloc_throughput/bench | 📈 view plot | 5.99 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 7.86 units |
| benchmarks/sequential/08_sort/bench | 📈 view plot | 6.18 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 5.41 units |
| benchmarks/sequential/13_soa_layout/bench | 📈 view plot | 6.11 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 746.80 units |
| benchmarks/server/03_pathological/server | 📈 view plot | 6.32 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 1,002.85 units |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+185
to
+189
| RegisterBcEmitter.new(fe, source: File.read(path), importer: imp).compile(program) | ||
| rescue StandardError, ScriptError | ||
| # Same accounting as lowering: reaching an unsupported arm still | ||
| # covered it. A file that lowers but does not emit stays "lowered". | ||
| nil |
| results = MiniVM::Golden.update_snapshots(root: dir, targets: [:stack]) | ||
| snapshot_path = File.join(fixture_dir, "return_i64.stack.bc") | ||
| results = MiniVM::Golden.update_snapshots(root: dir, targets: [:register]) | ||
| snapshot_path = File.join(fixture_dir, "return_i64.register.bc") |
|
|
||
| expect(results.map(&:status)).to eq([:written]) | ||
| expect(File.read(snapshot_path)).to include("instructions:\n0000 LOAD_CONST_I64") | ||
| expect(File.read(snapshot_path)).to include("register instructions:\n0000 ICONST r0 0") |
| FileUtils.mkdir_p(fixture_dir) | ||
| FileUtils.cp(source_path, File.join(fixture_dir, "return_i64.clear")) | ||
| snapshot_path = File.join(fixture_dir, "return_i64.stack.bc") | ||
| snapshot_path = File.join(fixture_dir, "return_i64.register.bc") |
| vm_target = "stack" if vm_target == "bc" | ||
| # The register machine is the only VM. --vm= is accepted and ignored so | ||
| # existing callers keep working. | ||
| ARGV.reject! { |arg| arg == "--vm" || arg =~ /\A--vm=\w+\z/ } |
| puts | ||
| puts " ruby examples/minivm/run_tests.rb --golden" | ||
| puts " Runs the stack/register VM golden harness specs" | ||
| puts " Runs the register VM golden harness specs" |
| puts " Runs the register VM golden harness specs" | ||
| puts | ||
| puts " ruby examples/minivm/run_tests.rb --vm=stack|register [tests...]" | ||
| puts " ruby examples/minivm/run_tests.rb --vm=register [tests...]" |
| end | ||
|
|
||
| def self.update_snapshots(root: File.join(ROOT, "examples", "minivm", "vm-tests"), targets: [:stack], check: false) | ||
| def self.update_snapshots(root: File.join(ROOT, "examples", "minivm", "vm-tests"), targets: [:register], check: false) |
Comment on lines
+185
to
+189
| RegisterBcEmitter.new(fe, source: File.read(path), importer: imp).compile(program) | ||
| rescue StandardError, ScriptError | ||
| # Same accounting as lowering: reaching an unsupported arm still | ||
| # covered it. A file that lowers but does not emit stays "lowered". | ||
| nil |
| program = lo.lower_program(fe.ast) | ||
| lowered += 1 | ||
| begin | ||
| RegisterBcEmitter.new(fe, source: File.read(path), importer: imp).compile(program) |
Diff Coverage BucketsDiff base:
Src Ruby Visibility BreakdownScope: Files: 213 Counts are nonblank, non-comment Ruby source lines. Protected methods are grouped into
Zig Special Coverage AlertsNo added production Zig lines require missing Loom/VOPR/wait-loop coverage alerts. |
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
3 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.
No description provided.