Skip to content

ZJIT: Count fallback reasons for set/get/definedivar#15324

Merged
tekknolagi merged 2 commits intoruby:masterfrom
tekknolagi:mb-setivar-in-shape
Nov 26, 2025
Merged

ZJIT: Count fallback reasons for set/get/definedivar#15324
tekknolagi merged 2 commits intoruby:masterfrom
tekknolagi:mb-setivar-in-shape

Conversation

@tekknolagi
Copy link
Contributor

@tekknolagi tekknolagi commented Nov 25, 2025 •
edited
Loading

lobsters:

Top-4 setivar fallback reasons (100.0% of total 7,789,008):
  shape_transition: 6,074,085 (78.0%)
   not_monomorphic: 1,484,013 (19.1%)
      not_t_object:   172,629 ( 2.2%)
       too_complex:    58,281 ( 0.7%)
Top-3 getivar fallback reasons (100.0% of total 9,348,832):
     not_t_object: 4,658,833 (49.8%)
  not_monomorphic: 4,542,316 (48.6%)
      too_complex:   147,683 ( 1.6%)
Top-3 definedivar fallback reasons (100.0% of total 366,383):
  not_monomorphic: 361,389 (98.6%)
      too_complex:   3,062 ( 0.8%)
     not_t_object:   1,932 ( 0.5%)

railsbench:

Top-3 setivar fallback reasons (100.0% of total 15,119,057):
  shape_transition: 13,760,763 (91.0%)
   not_monomorphic:    982,368 ( 6.5%)
      not_t_object:    375,926 ( 2.5%)
Top-2 getivar fallback reasons (100.0% of total 14,438,747):
     not_t_object: 7,643,870 (52.9%)
  not_monomorphic: 6,794,877 (47.1%)
Top-2 definedivar fallback reasons (100.0% of total 209,613):
  not_monomorphic: 209,526 (100.0%)
     not_t_object:      87 ( 0.0%)

shipit:

Top-3 setivar fallback reasons (100.0% of total 14,516,254):
  shape_transition: 8,613,512 (59.3%)
   not_monomorphic: 5,761,398 (39.7%)
      not_t_object:   141,344 ( 1.0%)
Top-2 getivar fallback reasons (100.0% of total 21,016,444):
  not_monomorphic: 11,313,482 (53.8%)
     not_t_object:  9,702,962 (46.2%)
Top-2 definedivar fallback reasons (100.0% of total 290,382):
  not_monomorphic: 287,755 (99.1%)
     not_t_object:   2,627 ( 0.9%)

@tekknolagi tekknolagi marked this pull request as ready for review November 25, 2025 19:10
@matzbot matzbot requested a review from a team November 25, 2025 19:10
@eregon
Copy link
Member

eregon commented Nov 25, 2025

Re getivar not_t_object,

diff --git i/zjit/src/hir.rs w/zjit/src/hir.rs
index fbc9d80700..a2e087e44c 100644
--- i/zjit/src/hir.rs
+++ w/zjit/src/hir.rs
@@ -2818,6 +2818,7 @@ impl Function {
                         if !recv_type.flags().is_t_object() {
                             // Check if the receiver is a T_OBJECT
                             self.push_insn(block, Insn::IncrCounter(Counter::getivar_fallback_not_t_object));
+                            println!("getivar_fallback_not_t_object {}", get_class_name(recv_type.class()));
                             self.push_insn_id(block, insn_id); continue;
                         }
                         if recv_type.shape().is_too_complex() {

reveals on railsbench:
During bundle install:

getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module

During the benchmark:

getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object ActiveSupport::Inflector::Inflections::Uncountables
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Class
getivar_fallback_not_t_object Module
getivar_fallback_not_t_object Module
...

So probably we should handle T_MODULE & T_CLASS too.

@tekknolagi
Copy link
Contributor Author

tekknolagi commented Nov 25, 2025

oh wow

@tekknolagi
Copy link
Contributor Author

tekknolagi commented Nov 25, 2025

You are welcome to tackle if you like!

@eregon
Copy link
Member

eregon commented Nov 25, 2025

ActiveSupport::Inflector::Inflections::Uncountables actually makes sense because on Rails 8.0.3:
https://github.com/rails/rails/blob/v8.0.3/activesupport/lib/active_support/inflector/inflections.rb#L33
already fixed on master:
https://github.com/rails/rails/blob/4c53863e354ad94dbfaf0643d9cdc99393a1a583/activesupport/lib/active_support/inflector/inflections.rb#L35

@eregon
Copy link
Member

eregon commented Nov 25, 2025

You are welcome to tackle if you like!

Will give it a shot. I'm not sure if T_MODULE & T_CLASS are handled the same way as T_OBJECT layout-wise, I'll take a look and ask if I don't find out clearly in the source.

@eregon eregon mentioned this pull request Nov 25, 2025
Merged
@eregon
Copy link
Member

eregon commented Nov 25, 2025

#15327

eregon
eregon approved these changes Nov 25, 2025
View reviewed changes
Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tekknolagi tekknolagi merged commit db94a79 into ruby:master Nov 26, 2025
89 of 91 checks passed
@tekknolagi tekknolagi deleted the mb-setivar-in-shape branch November 26, 2025 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eregon eregon eregon approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tekknolagi @eregon