From ed0465a8a94f0f7c844f5c0b1c9e9a345cab0982 Mon Sep 17 00:00:00 2001 From: Kevin Menard Date: Tue, 16 Jun 2026 17:12:11 -0400 Subject: [PATCH 1/4] ZJIT: Fix and improve the inliner codegen tests (#17365) The tests under `with_inlining` are meant to verify that inlining a callee preserves the method's semantics, but many of them called the entry method too few times to ever trigger compilation. With the test call threshold of 2, a method is compiled on its second call and that same call runs the freshly compiled body, so a test that called the entry method only once never crossed the threshold and never compiled it at all. Those tests passed purely on the interpreter's result, leaving the inliner untested while appearing to cover it. The new `assert_inlines` will help catch issues like this in the future. --- zjit/src/codegen_tests.rs | 101 +++++++++++++++++++++++++++++--------- zjit/src/options.rs | 7 +++ 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 6dc607e399de31..4eb60ee5db13ae 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -6,23 +6,65 @@ use crate::backend::lir::Assembler; use crate::codegen::max_iseq_versions; use crate::cruby::*; use crate::hir::{Insn, iseq_to_hir}; -use crate::options::{OPTIONS, get_option, rb_zjit_prepare_options, set_call_threshold}; +use crate::options::{get_option, rb_zjit_prepare_options, set_call_threshold, set_inline_threshold}; use crate::payload::IseqVersion; use crate::hir::tests::hir_build_tests::assert_contains_opcode; use crate::payload::*; use insta::assert_snapshot; +/// Run the Ruby fragment with the inliner enabled with the default inline +/// threshold for tests. Most inliner tests should use this. `with_inlining_threshold` +/// exists if you need to customize the inline threshold for a given test. #[track_caller] -fn with_inlining(mut f: impl FnMut() -> T) -> T { +fn with_inlining(ruby_fragment: impl FnMut() -> T) -> T { + // 30 will compile common, smaller methods while not compiling the whole world. + with_inlining_threshold(30, ruby_fragment) +} + +/// Run the Ruby fragment with the inliner enabled with the given inline `threshold`. +#[track_caller] +fn with_inlining_threshold(threshold: usize, mut ruby_fragment: impl FnMut() -> T) -> T { with_rubyvm(|| { - let old_threshold = get_option!(inline_threshold); - unsafe { OPTIONS.as_mut().unwrap().inline_threshold = 30; } - let result = f(); - unsafe { OPTIONS.as_mut().unwrap().inline_threshold = old_threshold; } + let old_inline_threshold = get_option!(inline_threshold); + let old_call_threshold = unsafe { crate::options::rb_zjit_call_threshold }; + + set_inline_threshold(threshold); + set_call_threshold(2); + let result = ruby_fragment(); + set_call_threshold(old_call_threshold); + set_inline_threshold(old_inline_threshold); + result }) } +/// Like `assert_compiles`, but also asserts that the program inlined at least one method +/// while running. Inliner tests must call the entry method enough times to cross the call +/// threshold, otherwise the method is never compiled and the test code ends up running in +/// interpreter. Asserting on `inline_method_count` fails the test in that case. +#[track_caller] +fn assert_inlines(program: &str) -> String { + let counters = crate::state::ZJITState::get_counters(); + let inline_count_before = counters.inline_method_count; + let result = assert_compiles(program); + assert!(counters.inline_method_count > inline_count_before, + "expected the program to inline at least one method, but inline_method_count did not increase"); + result +} + +/// Like `assert_inlines`, but tolerates side exits. Use for inliner tests whose +/// inlined body legitimately exits at runtime, such as a `break` that unwinds out +/// of a literal block. +#[track_caller] +fn assert_inlines_allowing_exits(program: &str) -> String { + let counters = crate::state::ZJITState::get_counters(); + let inline_count_before = counters.inline_method_count; + let result = assert_compiles_allowing_exits(program); + assert!(counters.inline_method_count > inline_count_before, + "expected the program to inline at least one method, but inline_method_count did not increase"); + result +} + #[test] fn test_breakpoint_hir_codegen() { rb_zjit_prepare_options(); @@ -5889,11 +5931,12 @@ fn test_send_block_unused_warning_emitted_from_jit() { #[test] fn test_inlined_method_returns_correct_value() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def add_one(x) = x + 1 def test(n) = add_one(n) test(2) + test(2) "), @"3"); }); } @@ -5901,7 +5944,7 @@ fn test_inlined_method_returns_correct_value() { #[test] fn test_inlined_method_deoptimizes_on_redefinition() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(x) = x + 1 def test(n) = callee(n) @@ -5918,7 +5961,7 @@ fn test_inlined_method_deoptimizes_on_redefinition() { #[test] fn test_inlined_method_survives_compact_between_calls() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(x) = x + 1 def test(n) = callee(n) @@ -5935,7 +5978,7 @@ fn test_inlined_method_survives_compact_between_calls() { #[test] fn test_inlined_method_survives_compact_during_call() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def trigger_compact = GC.compact def callee(x) trigger_compact @@ -5945,8 +5988,6 @@ fn test_inlined_method_survives_compact_during_call() { test(1) test(1) - - test(1) "), @"2"); }); } @@ -5954,11 +5995,12 @@ fn test_inlined_method_survives_compact_during_call() { #[test] fn test_inlined_method_with_required_keyword() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(x, y:) = x + y def test(n) = callee(n, y: 10) test(2) + test(2) "), @"12"); }); } @@ -5966,11 +6008,12 @@ fn test_inlined_method_with_required_keyword() { #[test] fn test_inlined_method_with_optional_keyword_supplied() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(x, y: 100) = x + y def test(n) = callee(n, y: 5) test(2) + test(2) "), @"7"); }); } @@ -5978,11 +6021,12 @@ fn test_inlined_method_with_optional_keyword_supplied() { #[test] fn test_inlined_method_with_optional_keyword_omitted() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(x, y: 100) = x + y def test(n) = callee(n) test(2) + test(2) "), @"102"); }); } @@ -5990,11 +6034,12 @@ fn test_inlined_method_with_optional_keyword_omitted() { #[test] fn test_inlined_method_with_reordered_keywords() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(a:, b:) = a - b def test = callee(b: 1, a: 10) test + test "), @"9"); }); } @@ -6002,11 +6047,12 @@ fn test_inlined_method_with_reordered_keywords() { #[test] fn test_inlined_method_with_keyword_default_using_prior_param() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" def callee(x, y: x * 100) = x + y def test(n) = callee(n) test(2) + test(2) "), @"202"); }); } @@ -6017,7 +6063,7 @@ fn test_inlined_method_with_rescue_caught_in_callee() { // callee. The runtime exception walker must find the rescue clause via the // inlined callee's CFP. with_inlining(|| { - assert_snapshot!(assert_compiles(r#" + assert_snapshot!(assert_inlines(r#" def callee(x) begin raise "boom" if x.negative? @@ -6029,6 +6075,7 @@ fn test_inlined_method_with_rescue_caught_in_callee() { def test(n) = callee(n) test(-1) + test(-1) "#), @"42"); }); } @@ -6038,7 +6085,7 @@ fn test_inlined_method_with_rescue_caught_in_caller() { // The callee re-raises and the caller catches the exception after unwinding // the inlined callee frame. with_inlining(|| { - assert_snapshot!(assert_compiles(r#" + assert_snapshot!(assert_inlines(r#" def callee(x) raise "boom" if x.negative? 0 @@ -6052,6 +6099,7 @@ fn test_inlined_method_with_rescue_caught_in_caller() { end test(-1) + test(-1) "#), @"99"); }); } @@ -6059,7 +6107,7 @@ fn test_inlined_method_with_rescue_caught_in_caller() { #[test] fn test_inlined_method_with_ensure_runs_on_propagation() { with_inlining(|| { - assert_snapshot!(assert_compiles(r##" + assert_snapshot!(assert_inlines(r##" $log = [] def callee(x) begin @@ -6078,6 +6126,7 @@ fn test_inlined_method_with_ensure_runs_on_propagation() { end end + test(-1) result = test(-1) "#{$log.first}: #{result}" "##), @r#""ensured: caught""#); @@ -6086,8 +6135,10 @@ fn test_inlined_method_with_ensure_runs_on_propagation() { #[test] fn test_inlined_method_with_retry_resumes_begin_block() { - with_inlining(|| { - assert_snapshot!(assert_compiles(r#" + // The begin/rescue/retry callee is larger than the default test inline budget, + // so raise the threshold enough for it to be inlined. + with_inlining_threshold(100, || { + assert_snapshot!(assert_inlines(r#" def callee(counter) begin counter[0] += 1 @@ -6100,6 +6151,7 @@ fn test_inlined_method_with_retry_resumes_begin_block() { def test(c) = callee(c) test([0]) + test([0]) "#), @"2"); }); } @@ -6107,7 +6159,7 @@ fn test_inlined_method_with_retry_resumes_begin_block() { #[test] fn test_inlined_method_with_super_call() { with_inlining(|| { - assert_snapshot!(assert_compiles(" + assert_snapshot!(assert_inlines(" class Parent def greet = 'hi' end @@ -6130,7 +6182,7 @@ fn test_inlined_method_with_block_break_across_inlined_boundary() { // A `break` from the literal block unwinds to the inlined callee's CFP, // where the callee's CATCH_TYPE_BREAK entry must match. with_inlining(|| { - assert_snapshot!(assert_compiles_allowing_exits(" + assert_snapshot!(assert_inlines_allowing_exits(" def callee(arr) arr.each do |x| break 7 if x > 5 @@ -6139,6 +6191,7 @@ fn test_inlined_method_with_block_break_across_inlined_boundary() { def test(a) = callee(a) test([1, 6, 99]) + test([1, 6, 99]) "), @"7"); }); } diff --git a/zjit/src/options.rs b/zjit/src/options.rs index d164ce7d43e668..141b872c42fa61 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -630,6 +630,13 @@ pub fn set_call_threshold(call_threshold: CallThreshold) { update_profile_threshold(); } +/// Update --zjit-inline-threshold for testing +#[cfg(test)] +pub fn set_inline_threshold(inline_threshold: InlineThreshold) { + rb_zjit_prepare_options(); + unsafe { OPTIONS.as_mut().unwrap().inline_threshold = inline_threshold; } +} + /// Enable --zjit-stats for testing #[cfg(test)] pub fn enable_zjit_stats() { From 9ec736d34d1ee5f19c193fe409dfdab7c7d3defa Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 17 Jun 2026 06:34:53 +0900 Subject: [PATCH 2/4] Don't fail pr-playground when github-pr-info artifact is missing The cross-workflow artifact upload from wasm.yml is frequently invisible to this workflow_run handler, so the hard throw turned every non-PR run red. Treat a missing artifact like the other best-effort skips (no Playground label, no successful build) and bail out quietly. The underlying artifact visibility in wasm.yml is a separate matter. --- .github/workflows/pr-playground.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-playground.yml b/.github/workflows/pr-playground.yml index dc4f075a384293..be7b73a37dd590 100644 --- a/.github/workflows/pr-playground.yml +++ b/.github/workflows/pr-playground.yml @@ -83,7 +83,8 @@ jobs: }); const artifact = artifacts.find(artifact => artifact.name == 'github-pr-info'); if (!artifact) { - throw new Error('Cannot find github-pr-info.txt artifact'); + core.warning('Cannot find github-pr-info.txt artifact; skipping'); + return null; } const { data } = await github.rest.actions.downloadArtifact({ @@ -99,6 +100,10 @@ jobs: } const prNumber = await derivePRNumber(); + if (!prNumber) { + core.info(`No PR number could be derived; skipping.`); + return; + } const { data: pr } = await github.rest.pulls.get({ owner: context.repo.owner, From 32bc4a016c9a424c75a4504185cf3dd132862e46 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 17 Jun 2026 06:34:58 +0900 Subject: [PATCH 3/4] Don't fail post-push on transient bugs.ruby-lang.org errors The changeset fetch is a best-effort trigger that Redmine also polls on its own, so a transient 503 from it should not turn the whole post-push job red while the load-bearing git.ruby-lang.org sync has already succeeded. Mark the step continue-on-error like the other non-critical steps already are. --- .github/workflows/post_push.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/post_push.yml b/.github/workflows/post_push.yml index e351c8c2866712..db52a5d80dd3a5 100644 --- a/.github/workflows/post_push.yml +++ b/.github/workflows/post_push.yml @@ -28,6 +28,7 @@ jobs: if: ${{ github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/ruby_') }} - name: Fetch changesets on bugs.ruby-lang.org + continue-on-error: true run: | curl "https://bugs.ruby-lang.org/sys/fetch_changesets?key=${REDMINE_SYS_API_KEY}" -s --fail-with-body -w '* status: %{http_code}\n' env: From a614959d426c6345f4381bc5362f8940ca558853 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 17 Jun 2026 08:14:08 +0900 Subject: [PATCH 4/4] Avoid timeout in test_dump_all_with_ractors File.readlines slurps each Ractor's whole ~8MB dump into memory at once. Because dump_all dumps the shared heap, the concurrent Ractors' readlines buffers feed back into each other's dumps, so peak memory grows superlinearly and the stop-the-world barrier thrashes past the 100s timeout on macOS/M:N. File.foreach keeps peak memory flat (NR=16: 2.96GB -> 47MB) while still parsing every line. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/objspace/test_objspace.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/objspace/test_objspace.rb b/test/objspace/test_objspace.rb index aacfba987d4177..5a36377ffe94e5 100644 --- a/test/objspace/test_objspace.rb +++ b/test/objspace/test_objspace.rb @@ -849,7 +849,7 @@ def test_dump_all_with_ractors Tempfile.create do |f| ObjectSpace.dump_all(output: f) f.close - File.readlines(f.path).each do |line| + File.foreach(f.path) do |line| JSON.parse(line) end end