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

Backporting Ruby 3.3 YJIT changes to 3.2-pshopify #525

Closed
k0kubun opened this issue Jul 13, 2023 · 10 comments
Closed

Backporting Ruby 3.3 YJIT changes to 3.2-pshopify #525

k0kubun opened this issue Jul 13, 2023 · 10 comments

Comments

@k0kubun
Copy link
Member

k0kubun commented Jul 13, 2023

Problem

We want to test Ruby 3.3 YJIT changes on Core and SFR, but Core and 99% of SFR use Ruby 3.2.

Solution

Gradually backport Ruby 3.3 YJIT changes that have been tested on SFR canary to 3.2-pshopify.

@casperisfine WDYT? Also, is it intentional that v3.2.2-pshopifyN is not a tag but a branch? (Do you need to update a past pshopify version after it's cut?)

@k0kubun
Copy link
Member Author

k0kubun commented Jul 13, 2023

For clarity, #523 is for future Ruby releases, and this issue is for Ruby 3.2 that is too late to be gemified.

@casperisfine
Copy link

WDYT?

So, keeping in short because this is a public repo, but:

  • Depends how much work it is to backport all this.
  • It's possible that it would be less work to free a 3.3.0 branch and just migrate to that.
  • Previously I was against it because it was hard to have our custom rubies in development
  • But now that https://github.com/Shopify/ruby-definitions is a thing, it's much less of a concern.
  • The last remaining concern is precompiled gems, most notably the Rust based ones, but it's probably workable.

v3.2.2-pshopifyN is not a tag but a branch?

It can be either. Tags would be cleaner for sure, but that's just how I tend to work on them.

Do you need to update a past pshopify version after it's cut?

No and we shouldn't.

@k0kubun
Copy link
Member Author

k0kubun commented Jul 13, 2023

Depends how much work it is to backport all this.
It's possible that it would be less work to free a 3.3.0 branch and just migrate to that.

I think our situation is that we need to take one of the following options:

  1. Give up testing YJIT outside SFR canary before it's released
  2. Backport all that to somewhere, whether it's pshopify or a branch in gemified YJIT in the future
  3. Solve the precompiled gems problem for unreleased Rubies

(1) is a last resort if (2) and (3) don't work. I'm okey with either of the rest. I assume we don't afford to solve (3) for random Ruby revisions. I wonder how difficult it is to solve (3) for preview/rc releases, which seems useful in any case, but right now I feel like (2) is worth exploring. If it turns out to be impractical, we can rely on (3) with preview/rc releases or wait until the gemified version.

Previously I was against it because it was hard to have our custom rubies in development
But now that https://github.com/Shopify/ruby-definitions is a thing, it's much less of a concern.

Thank you for publishing it 👍

The last remaining concern is precompiled gems, most notably the Rust based ones, but it's probably workable.

We should be careful not to change the ABI, yes. However, I think most YJIT patches don't come with the interpreter's struct changes, and non-YJIT changes are more prone to it. So I think it's workable.

It can be either. Tags would be cleaner for sure, but that's just how I tend to work on them.
No and we shouldn't.

Got it. I got confused after I saw #519, but I guess we're going to switch the base branch or just create pshopify5 with it instead merging the PR. Thanks for the information.

@casperisfine
Copy link

We should be careful not to change the ABI, yes.

My main concern with Rust gems is various compilation errors on developers machines, because you need the rust toolchain etc etc.

@k0kubun
Copy link
Member Author

k0kubun commented Jul 14, 2023

This issue is not about backporting changes in a Rust gem though. Does backporting current YJIT changes in Ruby master to 3.2-pshopify impact how third-party Rust gems are built?

@casperisfine
Copy link

Sorry, I think there is a misunderstanding. My comments are about upgrading to 3.3.0-dev rather than to backport more substantial patches to 3.2.x.

@eregon
Copy link

eregon commented Aug 7, 2023

3. Solve the precompiled gems problem for unreleased Rubies

I was thinking about that briefly in the context of ruby/prism#1206.
And it seems the ABI version for 3.3 is still 3.3.0+0, i.e., there was only a single ABI version since development on CRuby 3.3 started in December.
So in terms of how often we would need to recompile against the ABI it seems very rarely for CRuby.
The big change then would be the ability to rebuild the relevant precompiled gems whenever the dev ABI changes, and push the precompiled gems to rubygems.org or another gem server.
If to another gem server, then it wouldn't need manual help from the gem owner (to gem push), which seems necessary if this is expected to be done quickly after an ABI bump.
So maybe some other gem server which is dedicated to precompiled gems for Ruby dev versions and can read some config from a GitHub repo to know how to build a precompiled gem, and it could do so whenever the CRuby ABI changes?

@casperisfine
Copy link

@eregon the problem is how do you bust caches? You need to bump the gem version.

Even with a private server, if you just replace the existing version, the old binaries will stay in various caches.

Or am I missing something?

@eregon
Copy link

eregon commented Aug 8, 2023

You would use existing gem versions, and the precompiled gem would embed the Ruby ABI version in the filename.
I am not sure if it's already possible for precompiled gems to embed the ruby version, but it seems a good feature anyway e.g. for Windows where some gems already do require "#{RUBY_VERSION_MAJOR}.#{RUBY_VERSION_MINOR}/myextension.so" and workaround by including all of those in the same precompiled .gem file.

@k0kubun
Copy link
Member Author

k0kubun commented Aug 9, 2023

I think we already agreed on accepting Ruby 3.3 YJIT optimizations in Ruby 3.2-pshopify since we've been doing this. Closing. We could discuss other stuff in a separate issue.

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

No branches or pull requests

3 participants