Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YJIT: Propagate Array, Hash, and String classes #10323

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Mar 21, 2024

This PR allows YJIT to upgrade types in Context to Array, Hash, and String even if the object is not frozen. YJIT can now skip class checks when it's already known.

Because RBASIC_CLASS of an object can change when the object gets a singleton class (or calls IO#reopen), blocks skipping a class guard will be invalidated when any object of the class gets a singleton class, disabling the optimization for the class going forward.

This PR seems to impact the following benchmarks:

before: ruby 3.4.0dev (2024-03-21T18:14:25Z master 75d9019e6f) +YJIT [x86_64-linux]
after: ruby 3.4.0dev (2024-03-21T21:26:30Z yjit-singleton 982653a4fe) +YJIT [x86_64-linux]

------------  -----------  ----------  ----------  ----------  -------------  ------------
bench         before (ms)  stddev (%)  after (ms)  stddev (%)  after 1st itr  before/after
activerecord  46.4         0.7         45.1        0.7         1.00           1.03
lobsters      980.6        1.3         971.3       1.5         0.99           1.01
railsbench    2318.6       0.8         2289.7      0.8         1.01           1.01
rubocop       128.3        4.3         125.7       4.5         1.03           1.02
ruby-lsp      119.8        7.4         114.0       0.9         0.95           1.05
protoboeuf    44.0         6.0         42.5        6.5         1.04           1.04
------------  -----------  ----------  ----------  ----------  -------------  ------------

@k0kubun k0kubun marked this pull request as ready for review March 21, 2024 22:59
@k0kubun k0kubun requested a review from a team as a code owner March 21, 2024 22:59
Copy link
Member

@XrXr XrXr left a comment

Choose a reason for hiding this comment

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

Nice. It looks like once we see a singleton class, we insert guards like usual as a fallback. So everything is covered.

@@ -805,6 +806,7 @@ make_singleton_class(VALUE obj)
FL_SET(klass, FL_SINGLETON);
RBASIC_SET_CLASS(obj, klass);
rb_singleton_class_attached(klass, obj);
rb_yjit_invalidate_no_singleton_class(orig_class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice descriptive function name :)

Copy link
Contributor

@maximecb maximecb left a comment

Choose a reason for hiding this comment

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

Nicely done. Clean PR 👍 Great performance results 👌

@maximecb maximecb merged commit 46bf6ae into ruby:master Mar 25, 2024
98 checks passed
@k0kubun k0kubun deleted the yjit-singleton branch March 25, 2024 16:19
artur-intech pushed a commit to artur-intech/ruby that referenced this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants