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

Rust YJIT Support in ruby-build #1970

Closed
maximecb opened this issue May 6, 2022 · 18 comments · Fixed by #2005
Closed

Rust YJIT Support in ruby-build #1970

maximecb opened this issue May 6, 2022 · 18 comments · Fixed by #2005

Comments

@maximecb
Copy link
Contributor

maximecb commented May 6, 2022

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, and cargo 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:

  • Possibly ruby-build could check if rustc >= 1.60 is already installed, and set --enable-yjit if present
  • Maybe ruby-build could report to the user whether ruby is being built with or without YJIT support. If built without YJIT support, ruby-build could print a message explaining that rustc >= 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 the ruby-build maintainers make this happen if you are open to it.

Tagging my colleagues @byroot @XrXr @noahgibbs.

@koic
Copy link
Contributor

koic commented May 6, 2022

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 RUBY_CONFIGURE_OPTS option. ruby-build user may not notice that YJIT is not built by default in Ruby 3.2 because unlike Ruby 3.1.

@mislav
Copy link
Member

mislav commented May 10, 2022

  • Possibly ruby-build could check if rustc >= 1.60 is already installed, and set --enable-yjit if present

Like @koic already pointed out, wouldn't it be better that the user is in control over when they set --enable-yjit, especially since it has the Rust dependency? RUBY_CONFIGURE_OPTS was made for situations like these.

@maximecb
Copy link
Contributor Author

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.

@mislav
Copy link
Member

mislav commented May 11, 2022

Thanks for explaining!

  • Possibly ruby-build could check if rustc >= 1.60 is already installed, and set --enable-yjit if present

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?

  • Maybe ruby-build could report to the user whether ruby is being build with or without YJIT support.

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?

@byroot
Copy link
Contributor

byroot commented May 11, 2022

what are the user-facing drawbacks if someone compiles Ruby without support for it?

Not much if you use the interpreter, however if you try to pass the --jit flag to Ruby, it will error out (or print a warning?)

How important is it to the average Ruby user to know whether YJIT is enabled or not?

I'd say what is important is that they get a clear information on how to get it if they wish to.

@eregon
Copy link
Member

eregon commented Jul 11, 2022

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?
About every builtin C ext does such automatic detection so it seems the mechanics people are used to nowadays.

@XrXr
Copy link

XrXr commented Jul 11, 2022

We only look for rustc when the user configures --enable-yjit now.

@eregon
Copy link
Member

eregon commented Jul 11, 2022

Not much if you use the interpreter, however if you try to pass the --jit flag to Ruby, it will error out (or print a warning?)

Actually it will just use MJIT instead in that case it seems, --yjit OTOH warns:

$ ruby --jit -v -e itself 
ruby 3.2.0dev (2022-06-26T06:36:14Z master c2e37c8ff7) +MJIT [x86_64-linux]

$ ruby --yjit -e0   
ruby: warning: Ruby was built without YJIT support

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.

@maximecb
Copy link
Contributor Author

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.

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 👍

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.

Just to be sure, you mean you'd accept a PR for the following:

ruby-build could report to the user whether ruby is being built with or without YJIT support. If built without YJIT support, ruby-build could print a message explaining that rustc >= 1.60 is required to build YJIT.

?

@eregon
Copy link
Member

eregon commented Jul 12, 2022

Yes, exactly.
It could be another function like this (that's called from verify_openssl in the definitions):

ruby-build/bin/ruby-build

Lines 1208 to 1242 in c0d6e22

# Post-install check that the openssl extension was built.
build_package_verify_openssl() {
"$RUBY_BIN" -e '
manager = ARGV[0]
packages = {
"apt-get" => Hash.new {|h,k| "lib#{k}-dev" }.update(
"openssl" => "libssl-dev",
"zlib" => "zlib1g-dev"
),
"yum" => Hash.new {|h,k| "#{k}-devel" }.update(
"yaml" => "libyaml-devel"
)
}
failed = %w[openssl readline zlib yaml].reject do |lib|
begin
require lib
rescue LoadError
$stderr.puts "The Ruby #{lib} extension was not compiled."
end
end
if failed.size > 0
$stderr.puts "ERROR: Ruby install aborted due to missing extensions"
$stderr.print "Try running `%s install -y %s` to fetch missing dependencies.\n\n" % [
manager,
failed.map { |lib| packages.fetch(manager)[lib] }.join(" ")
] unless manager.empty?
$stderr.puts "Configure options used:"
require "rbconfig"; require "shellwords"
RbConfig::CONFIG.fetch("configure_args").shellsplit.each { |arg| $stderr.puts " #{arg}" }
exit 1
end
' "$(basename "$(type -p yum apt-get | head -1)")" >&4 2>&1
}

@noahgibbs
Copy link
Contributor

Opened a PR for this: #2005

@eregon
Copy link
Member

eregon commented Jul 13, 2022

Fixed by #2005

@eregon
Copy link
Member

eregon commented Nov 16, 2022

Since ruby/ruby#6662 was merged, maybe the logic in ruby-build can be simplified now, WDYT?

@byroot
Copy link
Contributor

byroot commented Nov 16, 2022

Definitely.

@byroot
Copy link
Contributor

byroot commented Nov 16, 2022

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.

@mislav
Copy link
Member

mislav commented Nov 16, 2022

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 enable_yjit step and associated implementation from ruby-build?

@eregon
Copy link
Member

eregon commented Nov 16, 2022

Right, we'd keep the warning: https://github.com/rbenv/ruby-build/pull/2005/files#diff-9161f3dbce993baa919f6535fbe4c32d2d2cd1ba9e5c93aa899188063c9022acR1277.
But for that we basically need the rest of the logic, and the only part which changes is then removing https://github.com/rbenv/ruby-build/pull/2005/files#diff-9161f3dbce993baa919f6535fbe4c32d2d2cd1ba9e5c93aa899188063c9022acR1274-R1275 because that's now automatic in ruby/ruby.

@mislav
Copy link
Member

mislav commented Nov 16, 2022

Keeping the warning means:

  • We have to forever duplicate the logic of discovering a compatible rustc on the system the same way that the Ruby build process does. If we fail to do so, our warning might become a false positive.
  • We have to keep an explicit enable_yjit step in the build-definition of every Ruby version >= 3.2 until the end of time.
  • I would wager that most Ruby users can't tell whether they “need” YJIT or not, unless they are deploying the app to production.

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 ./configure && make && make install.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants