-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ZJIT: add support for lazy RubyVM::ZJIT.enable
#15219
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
Conversation
7b9ea0f to
c72bdbd
Compare
|
|
||
| /// Enable ZJIT compilation. | ||
| fn zjit_enable() { | ||
| // TODO: call RubyVM::ZJIT::call_jit_hooks here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to fill this in here, but not sure what is the approach we want to use here.
The yjit equivalent does this:
let yjit = rb_const_get(rb_cRubyVM, rust_str_to_id("YJIT"));
rb_funcall(yjit, rust_str_to_id("call_jit_hooks"), 0);We have rb_funcallv in zjit, I suppose I could bind rb_cRubyVM and rb_const_get and make it work, just want to get someone else's opinion before I do that.
Alternatively, since this code will remain dormant anyway until Shopify#667 is resolved, we could also punt for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have rb_funcallv in zjit, I suppose I could bind rb_cRubyVM and rb_const_get and make it work, just want to get someone else's opinion before I do that.
Yeah, I think that's what we want to do. I don't come up with any better way.
37c559f to
82d7dac
Compare
This implements Shopify#854: - Splits boot-time and enable-time initialization, tracks progress with `InitializationState` enum - Introduces `RubyVM::ZJIT.enable` Ruby method for enabling the JIT lazily, if not already enabled - Introduces `--zjit-disable` flag, which can be used alongside the other `--zjit-*` flags but prevents enabling the JIT at boot time - Adds ZJIT infra to support JIT hooks, but this is not currently exercised (Shopify#667) Left for future enhancements: - Support kwargs for overriding the CLI flags in `RubyVM::ZJIT.enable` Closes Shopify#854
82d7dac to
1431ae0
Compare
k0kubun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks!
|
This implements Shopify#854:
Splits boot-time and enable-time initialization, tracks progress with
InitializationStateenumIntroduces
RubyVM::ZJIT.enableRuby method for enabling the JIT lazily, if not already enabledIntroduces
--zjit-disableflag, which can be used alongside the other--zjit-*flags but prevents enabling the JIT at boot timeAdds ZJIT infra to support JIT hooks, but this is not currently exercised (ZJIT: Allow ZJIT to use
#with_jithooks Shopify/ruby#667)Left for future enhancements:
RubyVM::ZJIT.enableCloses Shopify#854