From 08af07b297c0daf3393024f6e03768074d99d9da Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 27 Aug 2025 19:31:28 +0900 Subject: [PATCH 01/10] Add more `File.path` tests --- test/ruby/test_file_exhaustive.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/ruby/test_file_exhaustive.rb b/test/ruby/test_file_exhaustive.rb index b20b597256a125..222578be269581 100644 --- a/test/ruby/test_file_exhaustive.rb +++ b/test/ruby/test_file_exhaustive.rb @@ -197,12 +197,23 @@ def test_path [regular_file, utf8_file].each do |file| assert_equal(file, File.open(file) {|f| f.path}) assert_equal(file, File.path(file)) - o = Object.new - class << o; self; end.class_eval do - define_method(:to_path) { file } - end + o = Struct.new(:to_path).new(file) + assert_equal(file, File.path(o)) + o = Struct.new(:to_str).new(file) assert_equal(file, File.path(o)) end + + conv_error = ->(method, msg = "converting with #{method}") { + o = Struct.new(method).new(42) + assert_raise(TypeError, msg) {File.path(o)} + o = Struct.new(method).new("abc".encode(Encoding::UTF_32BE)) + assert_raise(Encoding::CompatibilityError, msg) {File.path(o)} + o = Struct.new(method).new("\0") + assert_raise(ArgumentError, msg) {File.path(o)} + } + + conv_error[:to_path] + conv_error[:to_str] end def assert_integer(n) From f7be2816797a993f0aeda04c645f268ddff9108c Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 27 Aug 2025 20:08:48 +0900 Subject: [PATCH 02/10] [DOC] Clarify the conversion by `File.path` --- file.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/file.c b/file.c index 3d8c800429af4d..ff755b4ac312d7 100644 --- a/file.c +++ b/file.c @@ -5196,6 +5196,22 @@ rb_file_s_extname(VALUE klass, VALUE fname) * File.path(File::NULL) #=> "/dev/null" * File.path(Pathname.new("/tmp")) #=> "/tmp" * + * If +path+ is not a String: + * + * 1. If it has the +to_path+ method, that method will be called to + * coerce to a String. + * + * 2. Otherwise, or if the coerced result is not a String too, the + * standard coersion using +to_str+ method will take place on that + * object. (See also String.try_convert) + * + * The coerced string must satisfy the following conditions: + * + * 1. It must be in an ASCII-compatible encoding; otherwise, an + * Encoding::CompatibilityError is raised. + * + * 2. It must not contain the NUL character (\0); otherwise, + * an ArgumentError is raised. */ static VALUE From c3c74e0d31c0c7327d2eb2c79b253d6500c6f2c0 Mon Sep 17 00:00:00 2001 From: Jun Aruga Date: Wed, 27 Aug 2025 13:35:18 +0100 Subject: [PATCH 03/10] CI: Drop Ubuntu s390x temporarily. We are seeing the not-starting s390x jobs. Until the following issue is fixed, we drop the s390x case. https://github.com/IBM/actionspz/issues/34 --- .github/workflows/ubuntu-ibm.yml | 193 ------------------------------- 1 file changed, 193 deletions(-) delete mode 100644 .github/workflows/ubuntu-ibm.yml diff --git a/.github/workflows/ubuntu-ibm.yml b/.github/workflows/ubuntu-ibm.yml deleted file mode 100644 index 619b11bb5e20d1..00000000000000 --- a/.github/workflows/ubuntu-ibm.yml +++ /dev/null @@ -1,193 +0,0 @@ -name: Ubuntu IBM -on: - push: - paths-ignore: - - 'doc/**' - - '**/man/*' - - '**.md' - - '**.rdoc' - - '**/.document' - - '.*.yml' - pull_request: - # Do not use paths-ignore for required status checks - # https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks - merge_group: - -concurrency: - group: ${{ github.workflow }} / ${{ startsWith(github.event_name, 'pull') && github.ref_name || github.sha }} - cancel-in-progress: ${{ startsWith(github.event_name, 'pull') }} - -permissions: - contents: read - -jobs: - make: - strategy: - matrix: - test_task: [check] - configure: [''] - arch: [''] - os: - # FIXME Comment out ppc64le due to failing tests on GitHub Actions - # ppc64le - # https://bugs.ruby-lang.org/issues/21534 - # - ubuntu-24.04-ppc64le - - ubuntu-24.04-s390x - # The ppc64le/s390x runners work only in the registered repositories. - # They don't work in forked repositories. - # https://github.com/IBM/actionspz/blob/main/docs/FAQ.md#what-about-forked-repos - upstream: - - ${{ github.repository == 'ruby/ruby' }} - exclude: - - os: ubuntu-24.04-ppc64le - upstream: false - - os: ubuntu-24.04-s390x - upstream: false - fail-fast: false - - env: - GITPULLOPTIONS: --no-tags origin ${{ github.ref }} - RUBY_DEBUG: ci - - runs-on: ${{ matrix.os || 'ubuntu-22.04' }} - - if: >- - ${{!(false - || contains(github.event.head_commit.message, '[DOC]') - || contains(github.event.pull_request.title, '[DOC]') - || contains(github.event.pull_request.labels.*.name, 'Documentation') - || (github.event_name == 'push' && github.event.pull_request.user.login == 'dependabot[bot]') - )}} - - steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - sparse-checkout-cone-mode: false - sparse-checkout: /.github - - - uses: ./.github/actions/setup/ubuntu - with: - arch: ${{ matrix.arch }} - - - uses: ruby/setup-ruby@a9bfc2ecf3dd40734a9418f89a7e9d484c32b990 # v1.248.0 - with: - ruby-version: '3.1' - bundler: none - if: ${{ !endsWith(matrix.os, 'arm') && !endsWith(matrix.os, 'ppc64le') && !endsWith(matrix.os, 's390x') }} - - # Avoid possible test failures with the zlib applying the following patch - # on s390x CPU architecture. - # https://github.com/madler/zlib/pull/410 - - name: Disable DFLTCC - run: echo "DFLTCC=0" >> $GITHUB_ENV - working-directory: - if: ${{ endsWith(matrix.os, 's390x') }} - - # A temporary workaround: Set HOME env to pass the step - # ./.github/actions/setup/directories. - # https://github.com/IBM/actionspz/issues/30 - - name: Set HOME env - run: | - echo "HOME: #{HOME}" - echo "HOME=$(ls -d ~)" >> $GITHUB_ENV - working-directory: - if: ${{ endsWith(matrix.os, 'ppc64le') || endsWith(matrix.os, 's390x') }} - - - uses: ./.github/actions/setup/directories - with: - srcdir: src - builddir: build - makeup: true - clean: true - dummy-files: ${{ matrix.test_task == 'check' }} - # Set fetch-depth: 10 so that Launchable can receive commits information. - fetch-depth: 10 - - - name: Run configure - env: - arch: ${{ matrix.arch }} - configure: ${{ matrix.configure }} - run: >- - $SETARCH ../src/configure -C --disable-install-doc ${configure:-cppflags=-DRUBY_DEBUG} - ${arch:+--target=$arch-$OSTYPE --host=$arch-$OSTYPE} - - - run: $SETARCH make prepare-gems - if: ${{ matrix.test_task == 'test-bundled-gems' }} - - - run: $SETARCH make - - - run: $SETARCH make hello - - - name: runirb - run: | - echo IRB::VERSION | $SETARCH make runirb RUNOPT="-- -f" - - - name: Set test options for skipped tests - run: | - set -x - TESTS="$(echo "${{ matrix.skipped_tests }}" | sed 's| |$$/ -n!/|g;s|^|-n!/|;s|$|$$/|')" - echo "TESTS=${TESTS}" >> $GITHUB_ENV - if: ${{ matrix.test_task == 'check' && matrix.skipped_tests }} - - - name: Set up Launchable - id: launchable - uses: ./.github/actions/launchable/setup - with: - os: ${{ matrix.os || 'ubuntu-22.04' }} - test-opts: ${{ matrix.configure }} - launchable-token: ${{ secrets.LAUNCHABLE_TOKEN }} - builddir: build - srcdir: src - continue-on-error: true - timeout-minutes: 3 - - # A temporary workaround: Skip user ground id test - # There is a mismatch between the group IDs of "id -g" and C function - # getpwuid(uid_t uid) pw_gid. - # https://github.com/IBM/actionspz/issues/31 - - name: Skip user group id test - run: | - sed -i.orig '/^ it "returns user group id" do/a\ skip' \ - ../src/spec/ruby/library/etc/struct_passwd_spec.rb - diff -u ../src/spec/ruby/library/etc/struct_passwd_spec.rb{.orig,} || : - if: ${{ endsWith(matrix.os, 'ppc64le') || endsWith(matrix.os, 's390x') }} - - - name: make ${{ matrix.test_task }} - run: | - test -n "${LAUNCHABLE_STDOUT}" && exec 1> >(tee "${LAUNCHABLE_STDOUT}") - test -n "${LAUNCHABLE_STDERR}" && exec 2> >(tee "${LAUNCHABLE_STDERR}") - - $SETARCH make -s ${{ matrix.test_task }} \ - ${TESTS:+TESTS="$TESTS"} \ - ${{ !contains(matrix.test_task, 'bundle') && 'RUBYOPT=-w' || '' }} - timeout-minutes: ${{ matrix.timeout || 40 }} - env: - RUBY_TESTOPTS: '-q --tty=no' - TEST_BUNDLED_GEMS_ALLOW_FAILURES: '' - PRECHECK_BUNDLED_GEMS: 'no' - LAUNCHABLE_STDOUT: ${{ steps.launchable.outputs.stdout_report_path }} - LAUNCHABLE_STDERR: ${{ steps.launchable.outputs.stderr_report_path }} - - - name: make skipped tests - run: | - $SETARCH make -s test-all TESTS="${TESTS//-n!\//-n/}" - env: - GNUMAKEFLAGS: '' - RUBY_TESTOPTS: '-v --tty=no' - if: ${{ matrix.test_task == 'check' && matrix.skipped_tests }} - continue-on-error: ${{ matrix.continue-on-skipped_tests || false }} - - - name: test-pc - run: | - DESTDIR=${RUNNER_TEMP-${TMPDIR-/tmp}}/installed - $SETARCH make test-pc "DESTDIR=$DESTDIR" - - - uses: ./.github/actions/slack - with: - label: ${{ matrix.test_task }} ${{ matrix.configure }}${{ matrix.arch }} - SLACK_WEBHOOK_URL: ${{ secrets.SIMPLER_ALERTS_URL }} # ruby-lang slack: ruby/simpler-alerts-bot - if: ${{ failure() }} - -defaults: - run: - working-directory: build From d2ef901f6cf86b3cd172782e2c629be162c99b72 Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Sun, 24 Aug 2025 10:37:26 -0500 Subject: [PATCH 04/10] [DOC] Tweaks for String#length --- doc/string/length.rdoc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/string/length.rdoc b/doc/string/length.rdoc index 544bca269f5511..5b302380b59fdb 100644 --- a/doc/string/length.rdoc +++ b/doc/string/length.rdoc @@ -2,11 +2,12 @@ Returns the count of characters (not bytes) in +self+: 'foo'.length # => 3 'тест'.length # => 4 - 'こんにちは'.length # => 5 + 'こんにちは'.length # => 5 Contrast with String#bytesize: 'foo'.bytesize # => 3 'тест'.bytesize # => 8 - 'こんにちは'.bytesize # => 15 + 'こんにちは'.bytesize # => 15 +Related: see {Querying}[rdoc-ref:String@Querying]. From 8935cf98e5e6b1b94ffcb6ab46c83339487f5de0 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 26 Aug 2025 15:42:41 -0400 Subject: [PATCH 05/10] Fix malloc_gc_disabled in Ractor lock We should disable GC for malloc for the current Ractor instead of the locked Ractor because it's the current Ractor that could run code that mallocs. --- ractor.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/ractor.c b/ractor.c index 57c9c4178f82d8..4322bf1246e9ed 100644 --- a/ractor.c +++ b/ractor.c @@ -71,7 +71,11 @@ ractor_lock(rb_ractor_t *r, const char *file, int line) ASSERT_ractor_unlocking(r); rb_native_mutex_lock(&r->sync.lock); - r->malloc_gc_disabled = true; + + if (rb_current_execution_context(false)) { + VM_ASSERT(!GET_RACTOR()->malloc_gc_disabled); + GET_RACTOR()->malloc_gc_disabled = true; + } #if RACTOR_CHECK_MODE > 0 if (rb_current_execution_context(false) != NULL) { @@ -101,9 +105,11 @@ ractor_unlock(rb_ractor_t *r, const char *file, int line) r->sync.locked_by = Qnil; #endif - VM_ASSERT(r->malloc_gc_disabled); + if (rb_current_execution_context(false)) { + VM_ASSERT(GET_RACTOR()->malloc_gc_disabled); + GET_RACTOR()->malloc_gc_disabled = false; + } - r->malloc_gc_disabled = false; rb_native_mutex_unlock(&r->sync.lock); RUBY_DEBUG_LOG2(file, line, "r:%u%s", r->pub.id, rb_current_ractor_raw(false) == r ? " (self)" : ""); From 47d4cceeff1be3c661af09fa2e29a3578dbab5e6 Mon Sep 17 00:00:00 2001 From: Jun Aruga Date: Tue, 26 Aug 2025 15:38:16 +0100 Subject: [PATCH 06/10] CI: Use `nproc` to count only on-line CPUs for GNUMAKEFLAGS Use `nproc` rather than `nproc --all`. Because the number by the `nproc` is different from the number by the `nproc --all` on GitHub Actions ppc64le/s390x. This caused the `make` command runs much more jobs than the number of on-line CPUs on these environments. The make command ran 193 jobs in parallel for 4 on-line CPUs in GitHub Actions ppc64le, and ran 9 jobs in parallel for 4 on-line CPUs in GitHub Actions s390x. And this caused the high load average 34.58 on ppc64le, and 6.69 on s390x. These values should be less than 4.0. I believe we should use the `nproc` rather than `nproc --all`. ``` + nproc 4 + nproc --all 192 ``` ``` + nproc 4 + nproc --all 8 ``` See https://github.com/IBM/actionspz/issues/38 for details. Note the `--all` option in the `nproc --all` was originally added to boost CPU in a hyper threading environment where one physical core works like two logical cores. https://www.intel.com/content/www/us/en/gaming/resources/hyper-threading.html --- .github/actions/compilers/entrypoint.sh | 2 +- .github/actions/setup/directories/action.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/compilers/entrypoint.sh b/.github/actions/compilers/entrypoint.sh index 17f749d69ebf60..4f9d1304ee2ec2 100755 --- a/.github/actions/compilers/entrypoint.sh +++ b/.github/actions/compilers/entrypoint.sh @@ -26,7 +26,7 @@ export CONFIGURE_TTY='never' export RUBY_DEBUG='ci rgengc' export RUBY_TESTOPTS='-q --color=always --tty=no' export RUBY_DEBUG_COUNTER_DISABLE='1' -export GNUMAKEFLAGS="-j$((1 + $(nproc --all)))" +export GNUMAKEFLAGS="-j$((1 + $(nproc)))" case "x${INPUT_ENABLE_SHARED}" in x | xno | xfalse ) diff --git a/.github/actions/setup/directories/action.yml b/.github/actions/setup/directories/action.yml index 728e0821891d4c..885eabf571d08a 100644 --- a/.github/actions/setup/directories/action.yml +++ b/.github/actions/setup/directories/action.yml @@ -110,7 +110,7 @@ runs: - if: runner.os == 'Linux' shell: bash - run: echo "GNUMAKEFLAGS=-sj$((1 + $(nproc --all)))" >> "$GITHUB_ENV" + run: echo "GNUMAKEFLAGS=-sj$((1 + $(nproc)))" >> "$GITHUB_ENV" # macOS' GNU make is so old that they doesn't understand `GNUMAKEFLAGS`. - if: runner.os == 'macOS' From 6c0315d99a93bdea947f821bd337000420ab41d1 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 26 Aug 2025 15:09:32 -0400 Subject: [PATCH 07/10] Rename rb_hook_list_mark_and_update to rb_hook_list_mark_and_move --- iseq.c | 2 +- vm_core.h | 2 +- vm_trace.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/iseq.c b/iseq.c index 09346994dd180b..ac59429901ce36 100644 --- a/iseq.c +++ b/iseq.c @@ -432,7 +432,7 @@ rb_iseq_mark_and_move(rb_iseq_t *iseq, bool reference_updating) VM_ASSERT(ISEQ_EXECUTABLE_P(iseq)); if (iseq->aux.exec.local_hooks) { - rb_hook_list_mark_and_update(iseq->aux.exec.local_hooks); + rb_hook_list_mark_and_move(iseq->aux.exec.local_hooks); } } diff --git a/vm_core.h b/vm_core.h index b5a04101abfcd0..972e6b267cde5d 100644 --- a/vm_core.h +++ b/vm_core.h @@ -2190,7 +2190,7 @@ struct rb_trace_arg_struct { }; void rb_hook_list_mark(rb_hook_list_t *hooks); -void rb_hook_list_mark_and_update(rb_hook_list_t *hooks); +void rb_hook_list_mark_and_move(rb_hook_list_t *hooks); void rb_hook_list_free(rb_hook_list_t *hooks); void rb_hook_list_connect_tracepoint(VALUE target, rb_hook_list_t *list, VALUE tpval, unsigned int target_line); void rb_hook_list_remove_tracepoint(rb_hook_list_t *list, VALUE tpval); diff --git a/vm_trace.c b/vm_trace.c index 4032b1f6391fa1..cb4feff147168e 100644 --- a/vm_trace.c +++ b/vm_trace.c @@ -71,7 +71,7 @@ rb_hook_list_mark(rb_hook_list_t *hooks) } void -rb_hook_list_mark_and_update(rb_hook_list_t *hooks) +rb_hook_list_mark_and_move(rb_hook_list_t *hooks) { rb_event_hook_t *hook = hooks->hooks; From 61d26c35bf8c744b4c59a44536bc58a6c4653ab6 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 26 Aug 2025 15:20:54 -0400 Subject: [PATCH 08/10] Don't pin method hooks of bmethods --- imemo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/imemo.c b/imemo.c index 0ffabe80183b6e..abf101f362f325 100644 --- a/imemo.c +++ b/imemo.c @@ -313,8 +313,8 @@ mark_and_move_method_entry(rb_method_entry_t *ment, bool reference_updating) break; case VM_METHOD_TYPE_BMETHOD: rb_gc_mark_and_move(&def->body.bmethod.proc); - if (!reference_updating) { - if (def->body.bmethod.hooks) rb_hook_list_mark(def->body.bmethod.hooks); + if (def->body.bmethod.hooks) { + rb_hook_list_mark_and_move(def->body.bmethod.hooks); } break; case VM_METHOD_TYPE_ALIAS: From 76810fc34905011535f50c3f8bbcaf39cb80b6cc Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 27 Aug 2025 10:01:07 -0700 Subject: [PATCH 09/10] ZJIT: Implement side exit stats (#14357) --- internal/vm.h | 2 +- test/ruby/test_zjit.rb | 4 +- vm_exec.c | 2 +- vm_insnhelper.h | 6 +- yjit/bindgen/src/main.rs | 2 +- yjit/src/cruby_bindings.inc.rs | 2 +- yjit/src/stats.rs | 8 +-- zjit.rb | 45 ++++++++++----- zjit/bindgen/src/main.rs | 2 +- zjit/src/backend/arm64/mod.rs | 2 +- zjit/src/backend/lir.rs | 26 +++++++-- zjit/src/codegen.rs | 100 ++++++++++++++++++++------------- zjit/src/cruby_bindings.inc.rs | 2 +- zjit/src/hir.rs | 4 +- zjit/src/state.rs | 35 ++++++++++-- zjit/src/stats.rs | 36 ++++++++++-- 16 files changed, 192 insertions(+), 86 deletions(-) diff --git a/internal/vm.h b/internal/vm.h index 3a99011c44b2d8..e5ed47afae0e29 100644 --- a/internal/vm.h +++ b/internal/vm.h @@ -81,7 +81,7 @@ VALUE rb_gccct_clear_table(VALUE); #if USE_YJIT || USE_ZJIT /* vm_exec.c */ -extern uint64_t rb_vm_insns_count; +extern uint64_t rb_vm_insn_count; #endif extern bool rb_free_at_exit; diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 28d25f88a40751..9296cd35227429 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1490,8 +1490,8 @@ def test_stats def test = 1 test [ - RubyVM::ZJIT.stats[:zjit_insns_count] > 0, - RubyVM::ZJIT.stats(:zjit_insns_count) > 0, + RubyVM::ZJIT.stats[:zjit_insn_count] > 0, + RubyVM::ZJIT.stats(:zjit_insn_count) > 0, ] }, stats: true end diff --git a/vm_exec.c b/vm_exec.c index 3d7c241a321bfa..bd8c170d7643fe 100644 --- a/vm_exec.c +++ b/vm_exec.c @@ -13,7 +13,7 @@ #if USE_YJIT || USE_ZJIT // The number of instructions executed on vm_exec_core. --yjit-stats and --zjit-stats use this. -uint64_t rb_vm_insns_count = 0; +uint64_t rb_vm_insn_count = 0; #endif #if VM_COLLECT_USAGE_DETAILS diff --git a/vm_insnhelper.h b/vm_insnhelper.h index 24bfbb8210b7d0..015edaed9d12ee 100644 --- a/vm_insnhelper.h +++ b/vm_insnhelper.h @@ -16,11 +16,11 @@ RUBY_EXTERN rb_serial_t ruby_vm_constant_cache_invalidations; RUBY_EXTERN rb_serial_t ruby_vm_constant_cache_misses; RUBY_EXTERN rb_serial_t ruby_vm_global_cvar_state; -#if YJIT_STATS || ZJIT_STATS // We want vm_insns_count only on stats builds. -// Increment vm_insns_count for --yjit-stats. We increment this even when +#if YJIT_STATS || ZJIT_STATS // We want vm_insn_count only on stats builds. +// Increment vm_insn_count for --yjit-stats. We increment this even when // --yjit or --yjit-stats is not used because branching to skip it is slower. // We also don't use ATOMIC_INC for performance, allowing inaccuracy on Ractors. -#define JIT_COLLECT_USAGE_INSN(insn) rb_vm_insns_count++ +#define JIT_COLLECT_USAGE_INSN(insn) rb_vm_insn_count++ #else #define JIT_COLLECT_USAGE_INSN(insn) // none #endif diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 61b6f233269269..dd0cb6dbf523d7 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -390,7 +390,7 @@ fn main() { .allowlist_function("rb_const_get") // From internal/vm.h - .allowlist_var("rb_vm_insns_count") + .allowlist_var("rb_vm_insn_count") // From include/ruby/internal/intern/vm.h .allowlist_function("rb_get_alloc_func") diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index ea51574fe77391..4cae138c9581a8 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1092,7 +1092,7 @@ extern "C" { elts: *const VALUE, ) -> VALUE; pub fn rb_vm_top_self() -> VALUE; - pub static mut rb_vm_insns_count: u64; + pub static mut rb_vm_insn_count: u64; pub fn rb_method_entry_at(obj: VALUE, id: ID) -> *const rb_method_entry_t; pub fn rb_callable_method_entry(klass: VALUE, id: ID) -> *const rb_callable_method_entry_t; pub fn rb_callable_method_entry_or_negative( diff --git a/yjit/src/stats.rs b/yjit/src/stats.rs index 5358d83ea4197f..ea6130973d22e8 100644 --- a/yjit/src/stats.rs +++ b/yjit/src/stats.rs @@ -789,8 +789,8 @@ fn rb_yjit_gen_stats_dict(key: VALUE) -> VALUE { set_stat_usize!(hash, "context_cache_bytes", crate::core::CTX_ENCODE_CACHE_BYTES + crate::core::CTX_DECODE_CACHE_BYTES); // VM instructions count - if rb_vm_insns_count > 0 { - set_stat_usize!(hash, "vm_insns_count", rb_vm_insns_count as usize); + if rb_vm_insn_count > 0 { + set_stat_usize!(hash, "vm_insns_count", rb_vm_insn_count as usize); } set_stat_usize!(hash, "live_iseq_count", rb_yjit_live_iseq_count as usize); @@ -861,8 +861,8 @@ fn rb_yjit_gen_stats_dict(key: VALUE) -> VALUE { set_stat_double!(hash, "avg_len_in_yjit", avg_len_in_yjit); // Proportion of instructions that retire in YJIT - if rb_vm_insns_count > 0 { - let total_insns_count = retired_in_yjit + rb_vm_insns_count; + if rb_vm_insn_count > 0 { + let total_insns_count = retired_in_yjit + rb_vm_insn_count; set_stat_usize!(hash, "total_insns_count", total_insns_count as usize); let ratio_in_yjit: f64 = 100.0 * retired_in_yjit as f64 / total_insns_count as f64; diff --git a/zjit.rb b/zjit.rb index b20c110046fd59..cf0896e107203e 100644 --- a/zjit.rb +++ b/zjit.rb @@ -29,9 +29,9 @@ def stats(key = nil) stats = Primitive.rb_zjit_stats(key) return stats if stats.nil? || !key.nil? - if stats.key?(:vm_insns_count) && stats.key?(:zjit_insns_count) - stats[:total_insns_count] = stats[:vm_insns_count] + stats[:zjit_insns_count] - stats[:ratio_in_zjit] = 100.0 * stats[:zjit_insns_count] / stats[:total_insns_count] + if stats.key?(:vm_insn_count) && stats.key?(:zjit_insn_count) + stats[:total_insn_count] = stats[:vm_insn_count] + stats[:zjit_insn_count] + stats[:ratio_in_zjit] = 100.0 * stats[:zjit_insn_count] / stats[:total_insn_count] end stats @@ -52,11 +52,13 @@ def stats_string :gc_time_ns, :invalidation_time_ns, - :total_insns_count, - :vm_insns_count, - :zjit_insns_count, + :side_exit_count, + :total_insn_count, + :vm_insn_count, + :zjit_insn_count, :ratio_in_zjit, ], buf:, stats:) + print_counters_with_prefix(prefix: 'exit_', prompt: 'side exit reasons', buf:, stats:, limit: 20) buf end @@ -70,9 +72,9 @@ def assert_compiles # :nodoc: private def print_counters(keys, buf:, stats:) - left_pad = keys.map(&:size).max + 1 + left_pad = keys.map { |key| key.to_s.sub(/_time_ns\z/, '_time').size }.max + 1 keys.each do |key| - # Some stats like vm_insns_count and ratio_in_zjit are not supported on the release build + # Some stats like vm_insn_count and ratio_in_zjit are not supported on the release build next unless stats.key?(key) value = stats[key] @@ -90,11 +92,28 @@ def print_counters(keys, buf:, stats:) end end - def print_counters_with_prefix(buf:, stats:, prefix:, prompt:) - keys = stats.keys.select { |key| key.start_with?(prefix) && stats[key] > 0 } - unless keys.empty? - buf << "#{prompt}:\n" - print_counters(keys, buf:, stats:) + def print_counters_with_prefix(buf:, stats:, prefix:, prompt:, limit: nil) + counters = stats.select { |key, value| key.start_with?(prefix) && value > 0 } + return if stats.empty? + + counters.transform_keys! { |key| key.to_s.delete_prefix!(prefix) } + left_pad = counters.keys.map(&:size).max + right_pad = counters.values.map { |value| number_with_delimiter(value).size }.max + total = counters.values.sum + count = counters.size + + counters = counters.to_a + counters.sort_by! { |_, value| -value } + counters = counters.first(limit) if limit + + buf << "Top-#{counters.size} " if limit + buf << "#{prompt}" + buf << " (%.1f%% of all #{count})" % (100.0 * counters.map(&:last).sum / total) if limit + buf << ":\n" + counters.each do |key, value| + padded_key = key.rjust(left_pad, ' ') + padded_value = number_with_delimiter(value).rjust(right_pad, ' ') + buf << " #{padded_key}: #{padded_value} (%4.1f%%)\n" % (100.0 * value / total) end end diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index ac1034199653dd..a95b8dcaaa8594 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -402,7 +402,7 @@ fn main() { .allowlist_function("rb_mod_name") // From internal/vm.h - .allowlist_var("rb_vm_insns_count") + .allowlist_var("rb_vm_insn_count") // From include/ruby/internal/intern/vm.h .allowlist_function("rb_get_alloc_func") diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index c60ec532856849..e243477ec8259c 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -1737,7 +1737,7 @@ mod tests { fn test_store_unserviceable() { let (mut asm, mut cb) = setup_asm(); // This would put the source into SCRATCH_REG, messing up the destination - asm.store(Opnd::mem(64, Opnd::Reg(Assembler::SCRATCH_REG), 0), 0x83902.into()); + asm.store(Opnd::mem(64, SCRATCH_OPND, 0), 0x83902.into()); asm.compile_with_num_regs(&mut cb, 0); } diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 1bb4cd024b5295..7e317d49913e1a 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -6,6 +6,7 @@ use crate::cruby::{Qundef, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_ use crate::hir::SideExitReason; use crate::options::{debug, get_option}; use crate::cruby::VALUE; +use crate::stats::exit_counter_ptr; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; @@ -1265,12 +1266,12 @@ impl Assembler // then load SCRATCH_REG into the destination when it's safe. if !old_moves.is_empty() { // Make sure it's safe to use SCRATCH_REG - assert!(old_moves.iter().all(|&(_, opnd)| opnd != Opnd::Reg(Assembler::SCRATCH_REG))); + assert!(old_moves.iter().all(|&(_, opnd)| opnd != SCRATCH_OPND)); // Move SCRATCH <- opnd, and delay reg <- SCRATCH let (reg, opnd) = old_moves.remove(0); new_moves.push((Assembler::SCRATCH_REG, opnd)); - old_moves.push((reg, Opnd::Reg(Assembler::SCRATCH_REG))); + old_moves.push((reg, SCRATCH_OPND)); } } new_moves @@ -1584,13 +1585,26 @@ impl Assembler } asm_comment!(self, "save cfp->pc"); - self.load_into(Opnd::Reg(Assembler::SCRATCH_REG), Opnd::const_ptr(pc)); - self.store(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_PC), Opnd::Reg(Assembler::SCRATCH_REG)); + self.load_into(SCRATCH_OPND, Opnd::const_ptr(pc)); + self.store(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_PC), SCRATCH_OPND); asm_comment!(self, "save cfp->sp"); - self.lea_into(Opnd::Reg(Assembler::SCRATCH_REG), Opnd::mem(64, SP, stack.len() as i32 * SIZEOF_VALUE_I32)); + self.lea_into(SCRATCH_OPND, Opnd::mem(64, SP, stack.len() as i32 * SIZEOF_VALUE_I32)); let cfp_sp = Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SP); - self.store(cfp_sp, Opnd::Reg(Assembler::SCRATCH_REG)); + self.store(cfp_sp, SCRATCH_OPND); + + if get_option!(stats) { + asm_comment!(self, "increment an exit counter"); + self.load_into(SCRATCH_OPND, Opnd::const_ptr(exit_counter_ptr(pc))); + let counter_opnd = if cfg!(target_arch = "aarch64") { // See arm64_split() + // Using C_CRET_OPND since arm64_emit uses both SCRATCH0 and SCRATCH1 for IncrCounter. + self.lea_into(C_RET_OPND, Opnd::mem(64, SCRATCH_OPND, 0)); + C_RET_OPND + } else { // x86_emit expects Opnd::Mem + Opnd::mem(64, SCRATCH_OPND, 0) + }; + self.incr_counter(counter_opnd, 1.into()); + } asm_comment!(self, "exit to the interpreter"); self.frame_teardown(&[]); // matching the setup in :bb0-prologue: diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 021db1c4c8ce54..2b71596a171272 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -9,7 +9,7 @@ use crate::invariants::{track_bop_assumption, track_cme_assumption, track_single use crate::gc::{append_gc_offsets, get_or_create_iseq_payload, get_or_create_iseq_payload_ptr, IseqPayload, IseqStatus}; use crate::state::ZJITState; use crate::stats::incr_counter; -use crate::stats::{counter_ptr, with_time_stat, Counter, Counter::compile_time_ns}; +use crate::stats::{counter_ptr, with_time_stat, Counter, Counter::{compile_time_ns, exit_compilation_failure}}; use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr}; use crate::backend::lir::{self, asm_comment, asm_ccall, Assembler, Opnd, Target, CFP, C_ARG_OPNDS, C_RET_OPND, EC, NATIVE_STACK_PTR, NATIVE_BASE_PTR, SCRATCH_OPND, SP}; use crate::hir::{iseq_to_hir, Block, BlockId, BranchEdge, Invariant, RangeType, SideExitReason, SideExitReason::*, SpecialObjectType, SpecialBackrefSymbol, SELF_PARAM_IDX}; @@ -73,45 +73,36 @@ pub extern "C" fn rb_zjit_iseq_gen_entry_point(iseq: IseqPtr, _ec: EcPtr) -> *co return std::ptr::null(); } - // Reject ISEQs with very large temp stacks. - // We cannot encode too large offsets to access locals in arm64. - let stack_max = unsafe { rb_get_iseq_body_stack_max(iseq) }; - if stack_max >= i8::MAX as u32 { - debug!("ISEQ stack too large: {stack_max}"); - return std::ptr::null(); - } - // Take a lock to avoid writing to ISEQ in parallel with Ractors. // with_vm_lock() does nothing if the program doesn't use Ractors. - let code_ptr = with_vm_lock(src_loc!(), || { - with_time_stat(compile_time_ns, || gen_iseq_entry_point(iseq)) - }); - - // Assert that the ISEQ compiles if RubyVM::ZJIT.assert_compiles is enabled - if ZJITState::assert_compiles_enabled() && code_ptr.is_null() { - let iseq_location = iseq_get_location(iseq, 0); - panic!("Failed to compile: {iseq_location}"); - } - - code_ptr -} + with_vm_lock(src_loc!(), || { + let cb = ZJITState::get_code_block(); + let mut code_ptr = with_time_stat(compile_time_ns, || gen_iseq_entry_point(cb, iseq)); + + if code_ptr.is_none() { + // Assert that the ISEQ compiles if RubyVM::ZJIT.assert_compiles is enabled + if ZJITState::assert_compiles_enabled() { + let iseq_location = iseq_get_location(iseq, 0); + panic!("Failed to compile: {iseq_location}"); + } -/// See [gen_iseq_entry_point_body]. This wrapper is to make sure cb.mark_all_executable() -/// is called even if gen_iseq_entry_point_body() partially fails and returns a null pointer. -fn gen_iseq_entry_point(iseq: IseqPtr) -> *const u8 { - let cb = ZJITState::get_code_block(); - let code_ptr = gen_iseq_entry_point_body(cb, iseq); + // For --zjit-stats, generate an entry that just increments exit_compilation_failure and exits + if get_option!(stats) { + code_ptr = gen_compilation_failure_counter(cb); + } + } - // Always mark the code region executable if asm.compile() has been used. - // We need to do this even if code_ptr is null because, whether gen_entry() or - // gen_function_stub() fails or not, gen_function() has already used asm.compile(). - cb.mark_all_executable(); + // Always mark the code region executable if asm.compile() has been used. + // We need to do this even if code_ptr is None because, whether gen_entry() + // fails or not, gen_iseq() may have already used asm.compile(). + cb.mark_all_executable(); - code_ptr.map_or(std::ptr::null(), |ptr| ptr.raw_ptr(cb)) + code_ptr.map_or(std::ptr::null(), |ptr| ptr.raw_ptr(cb)) + }) } /// Compile an entry point for a given ISEQ -fn gen_iseq_entry_point_body(cb: &mut CodeBlock, iseq: IseqPtr) -> Option { +fn gen_iseq_entry_point(cb: &mut CodeBlock, iseq: IseqPtr) -> Option { // Compile ISEQ into High-level IR let Some(function) = compile_iseq(iseq) else { incr_counter!(compilation_failure); @@ -283,7 +274,6 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Optio let insn = function.find(insn_id); if let Err(last_snapshot) = gen_insn(cb, &mut jit, &mut asm, function, insn_id, &insn) { debug!("ZJIT: gen_function: Failed to compile insn: {insn_id} {insn}. Generating side-exit."); - incr_counter!(failed_gen_insn); gen_side_exit(&mut jit, &mut asm, &SideExitReason::UnhandledInstruction(insn_id), &function.frame_state(last_snapshot)); // Don't bother generating code after a side-exit. We won't run it. // TODO(max): Generate ud2 or equivalent. @@ -419,10 +409,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio | &Insn::Throw { state, .. } | &Insn::ToArray { state, .. } | &Insn::ToNewArray { state, .. } - => { - incr_counter!(failed_gen_insn_unexpected); - return Err(state); - } + => return Err(state), }; assert!(insn.has_output(), "Cannot write LIR output of HIR instruction with no output: {insn}"); @@ -1378,6 +1365,15 @@ pub fn local_size_and_idx_to_bp_offset(local_size: usize, local_idx: usize) -> i /// Convert ISEQ into High-level IR fn compile_iseq(iseq: IseqPtr) -> Option { + // Reject ISEQs with very large temp stacks. + // We cannot encode too large offsets to access locals in arm64. + let stack_max = unsafe { rb_get_iseq_body_stack_max(iseq) }; + if stack_max >= i8::MAX as u32 { + debug!("ISEQ stack too large: {stack_max}"); + incr_counter!(failed_iseq_stack_too_large); + return None; + } + let mut function = match iseq_to_hir(iseq) { Ok(function) => function, Err(err) => { @@ -1508,7 +1504,7 @@ c_callable! { // Exit to the interpreter spill_stack(iseq, cfp, sp); - return ZJITState::get_exit_trampoline().raw_ptr(cb); + return ZJITState::get_exit_trampoline_with_counter().raw_ptr(cb); } // Otherwise, attempt to compile the ISEQ. We have to mark_all_executable() beyond this point. @@ -1518,7 +1514,7 @@ c_callable! { } else { // Exit to the interpreter spill_stack(iseq, cfp, sp); - ZJITState::get_exit_trampoline() + ZJITState::get_exit_trampoline_with_counter() }; cb.mark_all_executable(); code_ptr.raw_ptr(cb) @@ -1610,6 +1606,20 @@ pub fn gen_exit_trampoline(cb: &mut CodeBlock) -> Option { }) } +/// Generate a trampoline that increments exit_compilation_failure and jumps to exit_trampoline. +pub fn gen_exit_trampoline_with_counter(cb: &mut CodeBlock, exit_trampoline: CodePtr) -> Option { + let mut asm = Assembler::new(); + + asm_comment!(asm, "function stub exit trampoline"); + gen_incr_counter(&mut asm, exit_compilation_failure); + asm.jmp(Target::CodePtr(exit_trampoline)); + + asm.compile(cb).map(|(code_ptr, gc_offsets)| { + assert_eq!(gc_offsets.len(), 0); + code_ptr + }) +} + fn gen_push_opnds(jit: &mut JITState, asm: &mut Assembler, opnds: &[Opnd]) -> lir::Opnd { let n = opnds.len(); @@ -1666,6 +1676,18 @@ fn gen_string_concat(jit: &mut JITState, asm: &mut Assembler, strings: Vec result } +/// Generate a JIT entry that just increments exit_compilation_failure and exits +fn gen_compilation_failure_counter(cb: &mut CodeBlock) -> Option { + let mut asm = Assembler::new(); + gen_incr_counter(&mut asm, exit_compilation_failure); + asm.cret(Qundef.into()); + + asm.compile(cb).map(|(code_ptr, gc_offsets)| { + assert_eq!(0, gc_offsets.len()); + code_ptr + }) +} + /// Given the number of spill slots needed for a function, return the number of bytes /// the function needs to allocate on the stack for the stack frame. fn aligned_stack_bytes(num_slots: usize) -> usize { diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 12fc6b91fa630c..c804ecce863ada 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -835,7 +835,7 @@ unsafe extern "C" { elts: *const VALUE, ) -> VALUE; pub fn rb_vm_top_self() -> VALUE; - pub static mut rb_vm_insns_count: u64; + pub static mut rb_vm_insn_count: u64; pub fn rb_method_entry_at(obj: VALUE, id: ID) -> *const rb_method_entry_t; pub fn rb_callable_method_entry(klass: VALUE, id: ID) -> *const rb_callable_method_entry_t; pub fn rb_callable_method_entry_or_negative( diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index f69605c43dc80c..d269baf8848762 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -2878,9 +2878,9 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let exit_state = state.clone(); profiles.profile_stack(&exit_state); - // Increment zjit_insns_count for each YARV instruction if --zjit-stats is enabled. + // Increment zjit_insn_count for each YARV instruction if --zjit-stats is enabled. if get_option!(stats) { - fun.push_insn(block, Insn::IncrCounter(Counter::zjit_insns_count)); + fun.push_insn(block, Insn::IncrCounter(Counter::zjit_insn_count)); } // try_into() call below is unfortunate. Maybe pick i32 instead of usize for opcodes. diff --git a/zjit/src/state.rs b/zjit/src/state.rs index 948204a1e6b633..6608f1ea23c10f 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -1,10 +1,10 @@ -use crate::codegen::{gen_exit_trampoline, gen_function_stub_hit_trampoline}; -use crate::cruby::{self, rb_bug_panic_hook, rb_vm_insns_count, EcPtr, Qnil, VALUE}; +use crate::codegen::{gen_exit_trampoline, gen_exit_trampoline_with_counter, gen_function_stub_hit_trampoline}; +use crate::cruby::{self, rb_bug_panic_hook, rb_vm_insn_count, EcPtr, Qnil, VALUE, VM_INSTRUCTION_SIZE}; use crate::cruby_methods; use crate::invariants::Invariants; use crate::asm::CodeBlock; use crate::options::get_option; -use crate::stats::Counters; +use crate::stats::{Counters, ExitCounters}; use crate::virtualmem::CodePtr; #[allow(non_upper_case_globals)] @@ -24,6 +24,9 @@ pub struct ZJITState { /// ZJIT statistics counters: Counters, + /// Side-exit counters + exit_counters: ExitCounters, + /// Assumptions that require invalidation invariants: Invariants, @@ -36,6 +39,9 @@ pub struct ZJITState { /// Trampoline to side-exit without restoring PC or the stack exit_trampoline: CodePtr, + /// Trampoline to side-exit and increment exit_compilation_failure + exit_trampoline_with_counter: CodePtr, + /// Trampoline to call function_stub_hit function_stub_hit_trampoline: CodePtr, } @@ -93,13 +99,24 @@ impl ZJITState { let zjit_state = ZJITState { code_block: cb, counters: Counters::default(), + exit_counters: [0; VM_INSTRUCTION_SIZE as usize], invariants: Invariants::default(), assert_compiles: false, method_annotations: cruby_methods::init(), exit_trampoline, function_stub_hit_trampoline, + exit_trampoline_with_counter: exit_trampoline, }; unsafe { ZJIT_STATE = Some(zjit_state); } + + // With --zjit-stats, use a different trampoline on function stub exits + // to count exit_compilation_failure. Note that the trampoline code depends + // on the counter, so ZJIT_STATE needs to be initialized first. + if get_option!(stats) { + let cb = ZJITState::get_code_block(); + let code_ptr = gen_exit_trampoline_with_counter(cb, exit_trampoline).unwrap(); + ZJITState::get_instance().exit_trampoline_with_counter = code_ptr; + } } /// Return true if zjit_state has been initialized @@ -142,6 +159,11 @@ impl ZJITState { &mut ZJITState::get_instance().counters } + /// Get a mutable reference to side-exit counters + pub fn get_exit_counters() -> &'static mut ExitCounters { + &mut ZJITState::get_instance().exit_counters + } + /// Was --zjit-save-compiled-iseqs specified? pub fn should_log_compiled_iseqs() -> bool { get_option!(log_compiled_iseqs).is_some() @@ -179,6 +201,11 @@ impl ZJITState { ZJITState::get_instance().exit_trampoline } + /// Return a code pointer to the exit trampoline for function stubs + pub fn get_exit_trampoline_with_counter() -> CodePtr { + ZJITState::get_instance().exit_trampoline_with_counter + } + /// Return a code pointer to the function stub hit trampoline pub fn get_function_stub_hit_trampoline() -> CodePtr { ZJITState::get_instance().function_stub_hit_trampoline @@ -199,7 +226,7 @@ pub extern "C" fn rb_zjit_init() { rb_bug_panic_hook(); // Discard the instruction count for boot which we never compile - unsafe { rb_vm_insns_count = 0; } + unsafe { rb_vm_insn_count = 0; } // ZJIT enabled and initialized successfully assert!(unsafe{ !rb_zjit_enabled_p }); diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index b17edcb3a0823f..b754404a66e002 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -1,6 +1,6 @@ use std::time::Instant; -use crate::{cruby::*, options::get_option, state::zjit_enabled_p}; +use crate::{cruby::*, options::get_option, state::{zjit_enabled_p, ZJITState}}; macro_rules! make_counters { ( @@ -70,15 +70,17 @@ make_counters! { } // The number of times YARV instructions are executed on JIT code - zjit_insns_count, + zjit_insn_count, // failed_: Compilation failure reasons + failed_iseq_stack_too_large, failed_hir_compile, failed_hir_compile_validate, failed_hir_optimize, - failed_gen_insn, - failed_gen_insn_unexpected, failed_asm_compile, + + // exit_: Side exit reasons (ExitCounters shares the same prefix) + exit_compilation_failure, } /// Increase a counter by a specified amount @@ -95,6 +97,16 @@ macro_rules! incr_counter { } pub(crate) use incr_counter; +/// The number of side exits from each YARV instruction +pub type ExitCounters = [u64; VM_INSTRUCTION_SIZE as usize]; + +/// Return a raw pointer to the exit counter for the YARV instruction at a given PC +pub fn exit_counter_ptr(pc: *const VALUE) -> *mut u64 { + let opcode = unsafe { rb_vm_insn_addr2opcode((*pc).as_ptr()) }; + let exit_counters = ZJITState::get_exit_counters(); + unsafe { exit_counters.get_unchecked_mut(opcode as usize) } +} + /// Return a Hash object that contains ZJIT statistics #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> VALUE { @@ -134,8 +146,20 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> set_stat!(hash, &counter.name(), unsafe { *counter_ptr(counter) }); } - if unsafe { rb_vm_insns_count } > 0 { - set_stat!(hash, "vm_insns_count", unsafe { rb_vm_insns_count }); + // Set side exit stats + let exit_counters = ZJITState::get_exit_counters(); + let mut side_exit_count = 0; + for op_idx in 0..VM_INSTRUCTION_SIZE as usize { + let op_name = insn_name(op_idx); + let key_string = "exit_".to_owned() + &op_name; + let count = exit_counters[op_idx]; + side_exit_count += count; + set_stat!(hash, &key_string, count); + } + set_stat!(hash, "side_exit_count", side_exit_count); + + if unsafe { rb_vm_insn_count } > 0 { + set_stat!(hash, "vm_insn_count", unsafe { rb_vm_insn_count }); } hash From 886268856ba7c70a6eaf25eeb402e6ebed9e851e Mon Sep 17 00:00:00 2001 From: Luke Gruber Date: Wed, 27 Aug 2025 09:26:57 -0400 Subject: [PATCH 10/10] Fix bad NameError raised using sendforward instruction through vcall If you called a VCALL method and the method takes forwarding arguments and then you forward those arguments along using the sendforward instruction, the method_missing class was wrongly chosen as NameError instead of NoMethodError. This is because the VM looked at the CallInfo of the vcall and determined it needed to raise NameError. Now we detect that case and raise NoMethodError. Fixes [Bug #21535] --- test/ruby/test_nomethod_error.rb | 28 ++++++++++++++++++++++++++++ vm_insnhelper.c | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_nomethod_error.rb b/test/ruby/test_nomethod_error.rb index 6d413e63914f53..aa2a88b2d8f3fb 100644 --- a/test/ruby/test_nomethod_error.rb +++ b/test/ruby/test_nomethod_error.rb @@ -106,4 +106,32 @@ def name assert_match(/undefined method.+this_method_does_not_exist.+for.+Module/, err.to_s) end + + def test_send_forward_raises + t = EnvUtil.labeled_class("Test") do + def foo(...) + forward(...) + end + end + obj = t.new + assert_raise(NoMethodError) do + obj.foo + end + end + + # [Bug #21535] + def test_send_forward_raises_when_called_through_vcall + t = EnvUtil.labeled_class("Test") do + def foo(...) + forward(...) + end + def foo_indirect + foo # vcall + end + end + obj = t.new + assert_raise(NoMethodError) do + obj.foo_indirect + end + end end diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 408f464dab027f..1299de8666fed0 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -4214,7 +4214,7 @@ static enum method_missing_reason ci_missing_reason(const struct rb_callinfo *ci) { enum method_missing_reason stat = MISSING_NOENTRY; - if (vm_ci_flag(ci) & VM_CALL_VCALL) stat |= MISSING_VCALL; + if (vm_ci_flag(ci) & VM_CALL_VCALL && !(vm_ci_flag(ci) & VM_CALL_FORWARDING)) stat |= MISSING_VCALL; if (vm_ci_flag(ci) & VM_CALL_FCALL) stat |= MISSING_FCALL; if (vm_ci_flag(ci) & VM_CALL_SUPER) stat |= MISSING_SUPER; return stat;