YJIT: Add RubyVM::YJIT.enable #8705
Conversation
1c7444c to
885eace
Compare
| void | ||
| Init_ruby_description(ruby_cmdline_options_t *opt) | ||
| { | ||
| const char *const jit_opt = | ||
| RJIT_OPTS_ON ? " +RJIT" : | ||
| YJIT_OPTS_ON ? YJIT_DESCRIPTION : | ||
| ""; | ||
| define_ruby_description(jit_opt); | ||
| } | ||
|
|
||
| void | ||
| ruby_set_yjit_description(void) | ||
| { | ||
| rb_const_remove(rb_cObject, rb_intern("RUBY_DESCRIPTION")); | ||
| define_ruby_description(YJIT_DESCRIPTION); | ||
| } | ||
|
|
There was a problem hiding this comment.
The PR looks good and simplifies a lot of things 👍
This is the only part I'm not sure about. It's not fully clear what the Ruby description is going to print. Does it only print +YJIT if YJIT is enabled, which can happen later during execution?
I feel like ideally, the description should tell us that this is a YJIT-capable build of CRuby, so maybe it should say something like +YJIT (enabled) or +YJIT (disabled) ? Otherwise, as a simpler solution, we could just always have +YJIT, and people would have to query RubyVM::YJIT::enabled? if they want to know if YJIT is enabled or not.
There was a problem hiding this comment.
Does it only print +YJIT if YJIT is enabled, which can happen later during execution?
Correct.
This PR doesn't change the format. If disabled, that part is "". If enabled, that part is "+YJIT", "+YJIT dev", "+YJIT dev_nodebug", or "+YJIT stats". The whole point of this PR's design is to avoid confusion, so I wouldn't introduce "+YJIT (disabled)" (also that state is in fact the same as just running the interpreter).
There was a problem hiding this comment.
The thing that may be confusing is that the "+YJIT" is going to appear only when YJIT is enabled, which is going to happen much later if YJIT is enabled in software. You won't see it if you run ruby -v.
That's why I think YJIT enabled and YJIT disabled make sense to report. It lets us know we have a YJIT-capable build of CRuby.
There was a problem hiding this comment.
I should have asked this first: what problem do you want to solve with this? Does it really require RUBY_DESCRIPTION to have +YJIT? If you're concerned about possibly breaking RubyVM::YJIT.enable when Ruby is not built with --enable-yjit and troubleshooting it is hard, for example, can we just tell that it's not built with --enable-yjit when RubyVM::YJIT.enable is called? If you want to quickly check if it's built with --enable-yjit, why not just run ruby --yjit -v?
In CRuby, we generally present build configurations in RbConfig::CONFIG instead of RUBY_DESCRIPTION. RbConfig::CONFIG["YJIT_SUPPORT"] is "yes" or "no". Can we use that information in RubyVM::YJIT APIs or warning/error messages to deal with the problem? Would you like to introduce RubyVM::YJIT.supported? to wrap it?
The thing that may be confusing
To me, what seems more confusing to end users is "+YJIT (disabled)". "+YJIT" seems to say YJIT is enabled, but "(disabled)" also says it's disabled. "+YJIT (disabled)" seems just as confusing as "YJIT is enabled but paused".
I think the "+XXX" format is generally used when it's actually enabled, not when it's capable of doing it. For example, JRuby prints +indy and +jit when the flags are passed, and doesn't print +indy (disabled) or +jit (disabled) when built with that support but disabled.
Similarly, today's CRuby prints +MN when RUBY_MN_THREADS=1 is given, and doesn't print +MN (disabled) just by the fact that USE_MN_THREADS macro is 1 when it's disabled. It seems inconsistent in CRuby alone too.
One last note, changing the format of RUBY_DESCRIPTION for the YJIT-disabled mode might need discussions on bugs.ruby-lang.org and Matz's approval unlike RubyVM::YJIT-closed changes like the current diff. I would rather merge this first without breaking compatibility to make sure we land this feature in Ruby 3.3, and then spend time discussing the format of RUBY_DESCRIPTION, which is less important than adding RubyVM::YJIT.enable in 3.3.
There was a problem hiding this comment.
I think it would be useful to know if we're running a YJIT-capable version of Ruby when doing ruby -v, that would be the use case, but I get that you don't want to break convention with other flags shown in the Ruby description.
There was a problem hiding this comment.
I've always wanted a quick way to check if --enable-shared because it does slow down the interpreter and macOS and ruby-build unfortunately turn it on by default. I wish ruby -v printed it, so I guess I get your use case.
I would still want something less confusing than +YJIT (disable), though. If there's no concise way to communicate it in ruby -v, maybe we could make ruby -vv more verbose and let it print important build configurations like YJIT_SUPPORT and ENABLE_SHARED. Either way, we'd need a ticket to introduce such changes. Until then, ruby -v --yjit seems like a good enough compromise.
Having this variable actually helps the performance of non-JITed calls. ----- ----------- ---------- ---------- ---------- ------------- ------------ bench before (ms) stddev (%) after (ms) stddev (%) after 1st itr before/after fib 241.9 0.5 225.4 1.0 1.06 1.07 ----- ----------- ---------- ---------- ---------- ------------- ------------ (benchmarked with --yjit-cold-threshold=0)
There was many public reports of 15-25% latency improvements for Rails apps that did enable Ruby 3.2 YJIT, and in 3.3 it's even better. Following ruby/ruby#8705, in Ruby 3.3 YJIT is paused instead of disabled by default, allowing us to enable it from an initializer.
|
Seeing Regarding |
There was many public reports of 15-25% latency improvements for Rails apps that did enable Ruby 3.2 YJIT, and in 3.3 it's even better. Following ruby/ruby#8705, in Ruby 3.3 YJIT is paused instead of disabled by default, allowing us to enable it from an initializer.
Background
Changes
RubyVM::YJIT.enablethat can enable YJIT even if it wasn't enabled at bootrb_jit_conteven if JIT is disabled.RUBY_DESCRIPTIONgets+YJITwhen this method is called.RubyVM::YJIT.resumeand rename--yjit-pauseto--yjit-disableRubyVM::YJIT.enabled?returnstruewhen it's paused, even though it was the intended behavior for us. To avoid such confusion, this PR gets rid of "resume"/"pause" terminology and only leaves "enable"/"disable".--yjitorRUBY_YJIT_ENABLE=1. You may sometimes need to specify YJIT tuning options while disabling YJIT at boot, and--yjit-disablecan be used for that purpose (e.g.--yjit-call-threshold=1 --yjit-disable).Benchmark
No significant performance impact on the interpreter.