-
Notifications
You must be signed in to change notification settings - Fork 780
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
Rust YJIT Support in ruby-build
#1970
Comments
JFYI, I have Rust YJIT installed by the following workaround command. % rustc --version
rustc 1.60.0 (7737e0b5c 2022-04-04)
% RUBY_CONFIGURE_OPTS=--enable-yjit rbenv install 3.2.0-dev
(snip)
% ruby --yjit -ve 'p RubyVM::YJIT.enabled?'
ruby 3.2.0dev (2022-05-06T09:13:22Z master 67950a4c0a) +YJIT [x86_64-darwin19]
true This is a current method as a user. It would be useful if an installation (or information) would be provided upstream without requiring the |
Like @koic already pointed out, wouldn't it be better that the user is in control over when they set |
The key difference is that YJIT was enabled by default (on all supported platforms) with Ruby 3.1. As of now, with Ruby head and Ruby 3.2, it will only be opt-in and only available if the Rust toolchain is installed. Unfortunately, I'm sure many users won't be aware of this. I was hoping that we could help make the users aware of the difference and maybe point them to some quick instructions on how to build Ruby with YJIT. Hold their hand a little bit. |
Thanks for explaining!
I like this approach. In usage instructions for ruby-build, we could suggest that users install rustc when compiling Ruby 3.2+. However, I wouldn't make Rust a requirement, and I definitely wouldn't automatically install rustc on their behalf if it's not already present on the system. As someone who is not familiar with YJIT, what are the user-facing drawbacks if someone compiles Ruby without support for it?
That's another viable option. For example, we do print a warning to the user if their Ruby version was built without openssl support, since we assume that most usages of Ruby will sooner or later need to make HTTPS requests. How important is it to the average Ruby user to know whether YJIT is enabled or not? |
Not much if you use the interpreter, however if you try to pass the
I'd say what is important is that they get a clear information on how to get it if they wish to. |
I'm thinking we could update https://github.com/rbenv/ruby-build/wiki to include rustc, maybe as a separate section if we don't want to suggest it as default since it's not "a required dependency" and not needed for older Ruby versions. I thought CRuby automatically detects if a recent enough rustc is around, does it not do that anymore? |
We only look for |
Actually it will just use MJIT instead in that case it seems,
I think the second idea is fine (and should be very easy to implement), PR welcome. The first idea seems more risky because e.g. it could potentially cause extra compilation problems, etc for something the user did not opt-in explicitly. |
Seems like a good idea to update the wiki and mention how to install the rust toolchain in there, and that it's necessary to build YJIT 👍
Just to be sure, you mean you'd accept a PR for the following:
? |
Yes, exactly. Lines 1208 to 1242 in c0d6e22
|
Opened a PR for this: #2005 |
Fixed by #2005 |
Since ruby/ruby#6662 was merged, maybe the logic in ruby-build can be simplified now, WDYT? |
Definitely. |
I think the YJIT team would still like to warn users if Ruby is compiled without YJIT, so it would be nice to keep that part (if the ruby-build maintainers aren't opposed) but other than that, #2005 can pretty much be reverted. |
From what I understand, enabling YJIT using rustc found on the system is now part of the standard build process in Ruby main branch and we can remove the |
Right, we'd keep the warning: https://github.com/rbenv/ruby-build/pull/2005/files#diff-9161f3dbce993baa919f6535fbe4c32d2d2cd1ba9e5c93aa899188063c9022acR1277. |
Keeping the warning means:
I do not strongly feel that this is a responsibility that ruby-build should keep. If the Ruby build process wants to warn the user about missing rustc in its own build log, it can do that and we can do better to surface that warning on stderr even when writing the build log to a file, but I don't feel we should be doing anything too special with Ruby 3.2+ build definitions other than |
First of all thank you for creating the incredibly useful tool that is ruby-build 🙏
As you may know, we have been working on porting YJIT to Rust, and this has now been upstreamed: ruby/ruby#5826
Rust YJIT requires
rustc
1.60 to be installed to build in release mode, andcargo
to build in dev/debug mode. As such, by default, CRuby will build without YJIT support. In order to build YJIT,--enable-yjit
has to be specified when running./configure
. More info on how to build YJIT can be found in the YJIT readme.Given that
ruby-build
is the primary way that many people deploy Ruby and a central part of the Ruby ecosystem, we would really like to have the tool support building Rust YJIT. This would really help lower the bar for people to get YJIT running and could truly help YJIT see more use in the wild.My colleague @byroot has told me that the
ruby-build
maintainers don't feel comfortable automatically installing the Rust toolchain. I understand your concerns with respect to security (and maybe the size of the download required). I was wondering if maybe we could explore other options.Some ideas that come to mind:
ruby-build
could check ifrustc
>= 1.60 is already installed, and set--enable-yjit
if presentruby-build
could report to the user whetherruby
is being built with or without YJIT support. If built without YJIT support,ruby-build
could print a message explaining thatrustc
>= 1.60 is required to build YJIT.I think we're likely going to freeze the
rustc
dependency at version 1.60 for some time so this requirement is going to be pretty stable. Happy to discuss pros and cons of various options and how we can help theruby-build
maintainers make this happen if you are open to it.Tagging my colleagues @byroot @XrXr @noahgibbs.
The text was updated successfully, but these errors were encountered: