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
Merge YJIT: an in-process JIT compiler #4992
Conversation
Is there any motivation to support |
Primarily so we can activate YJIT or stats without having to worry about preserving/changing RUBYOPT, which may contain other options. |
It's more convenient for some of the build systems we use in production, where it can be tricky to change command-line options for some ruby builds and not others. |
got it. Another point: I see the following when I try to switch it to "Rebase and merge": Given that ruby/ruby disables merge commits, the only remaining option is "Squash and merge". Would that be okay for you, or would you like to take a look at making it possible to use "Rebase and merge" to keep commit history? |
I tried the rebase locally, and it succeeded. It's probably just a limitation of GitHub. We can manually rebase and push it. |
yjit_init_codegen(); | ||
|
||
// YJIT Ruby module | ||
mYjit = rb_define_module("YJIT"); |
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.
Can we put this under RubyVM::
? People generally prefer putting CRuby-specific components under RubyVM
. I think we don't expect it to be ported to JRuby/TruffleRuby at least.
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 thought it was convenient to have a YJIT
module for YJIT-specific things. Typical use case:
YJIT.reset_stats! if defined?(YJIT.reset_stats!)
Do you mean we should move this into RubyVM::YJIT
? If this is the preferred way, we will make that change as requested after the merge.
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.
Do you mean we should move this into
RubyVM::YJIT
?
Yes.
I think that change makes sense.
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.
Given that RubyVM::MJIT
was approved "because RubyVM is just for internal features" https://bugs.ruby-lang.org/issues/14830#note-3, I just thought it'd be easier to get the approval of RubyVM::YJIT
than top-level YJIT
, but you can try proposing it without RubyVM::
this time.
I think we can skip the discussion for the preview release, however, before the actual 3.1 release, we may need another ticket on https://bugs.ruby-lang.org to discuss user-facing Ruby interface changes. We usually discuss such changes (e.g. YJIT
and its methods, the command-line options, and the environment variables) on https://bugs.ruby-lang.org/ before merging them.
P.S. You may want to put something at https://rubygems.org/gems/yjit to reserve the namespace. (e.g. https://rubygems.org/gems/ractor https://rubygems.org/gems/mjit)
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.
I think we're totally ok with moving the module into RubyVM::YJIT
to match existing conventions. It won't be a problem. We should be able to open a PR for that in the coming week.
I think it's good enough for the first merge. We can address minor issues after that, including my above comment. Now that Matz gave the approval to make you a committer, how would you like to finish merging it? We can either wait for you to go through https://bugs.ruby-lang.org/projects/ruby/wiki/CommitterHowto#What-to-do-for-registering-you-as-a-committer and let you merge it yourself, or let me push them to unblock the preview release early while doing the process in case it takes a long time. |
Hello @k0kubun. Thank you for reviewing! Happy to see that we are passing the CI tests. We just found that there is a bug in our keyword argument handling in YJIT that shows up in production. We are hoping to fix this today and update this PR, add another test. If we can't fix it today we will just disable the feature and still be able to merge today. I am ok with you merging the PR 👍 Is it ok if we take a few hours to look into the keyword argument problem today? |
Sounds good 👍 Thank you. edit: For the record, Alan contacted me about taking over the merge and I happily handed it over to him 🙂 |
15c604b
to
4c0fe34
Compare
In jit_guard_known_klass whenever we encounter a new class should recompile the current instruction. However, previously once jit_guard_known_klass had guarded for a heap object it would not recompile for any immediate (special const) objects arriving afterwards and would take a plain side-exit instead of a chain guard. This commit uses jit_chain_guard inside jit_guard_known_klass instead of the plain side exit, so that we can recompile for any special constants arriving afterwards.
This adds guards
Basically the same thing as opt_mod, but for multiplying.
This script is an lldb helper that just loops through all the comments stored and prints out the comment along with the address corresponding to the comment. For example, I'm crashing in JIT code at address 0x0000000110000168. Using the `lc` helper I can see that it's probably crashing inside the exit back to the interpreter ``` (lldb) bt 5 * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x22220021) frame #0: 0x0000000110000168 * frame #1: 0x00000001002b5ff5 miniruby`invoke_block_from_c_bh [inlined] invoke_block(ec=0x0000000100e05350, iseq=0x0000000100c1ff10, self=0x0000000100c76cc0, captured=<unavailable>, cref=0x0000000000000000, type=<unavailable>, opt_pc=<unavailable>) at vm.c:1268:12 frame #2: 0x00000001002b5f7d miniruby`invoke_block_from_c_bh [inlined] invoke_iseq_block_from_c(ec=<unavailable>, captured=<unavailable>, self=0x0000000100c76cc0, argc=2, argv=<unavailable>, kw_splat=0, passed_block_handler=0x0000000000000000, cref=0x0000000000000000, is_lambda=<unavailable>, me=0x0000000000000000) at vm.c:1340 frame #3: 0x00000001002b5e14 miniruby`invoke_block_from_c_bh(ec=<unavailable>, block_handler=<unavailable>, argc=<unavailable>, argv=<unavailable>, kw_splat=0, passed_block_handler=0x0000000000000000, cref=0x0000000000000000, is_lambda=<unavailable>, force_blockarg=0) at vm.c:1358 frame #4: 0x000000010029860b miniruby`rb_yield_values(n=<unavailable>) at vm_eval.c:0 (lldb) lc 0x11000006d "putobject_INT2FIX_1_" 0x110000083 "leave" 0x110000087 "check for interrupts" 0x110000087 "RUBY_VM_CHECK_INTS(ec)" 0x110000098 "check for finish frame" 0x1100000ed "getlocal_WC_0" 0x110000107 "getlocal_WC_1" 0x11000012a "opt_send_without_block" 0x110000139 "opt_send_without_block" 0x11000013c "exit to interpreter" ```
The last parameter to rb_struct_define_under needs to be NULL otherwise we can get a SEGV.
We clear locals when we know their values might change (ex. when performing a method call). However previously values on the stack which were originally pushed from a local would still point back to that local. With this commit, when clearing locals, we'll now iterate over the mappings of the stack and copy the known type from the local to the stack mapping, removing the association to the local. This should mean both that we'll retain any information we already know about the local type, and that if a local is modified we won't incorrectly infer it's new type from the existing value on the stack.
Since opt_getinlinecache and opt_setinlinecache point to the same cache struct, there is no need to track the index of the get instruction and then store it on the cache struct later when processing the set instruction. Setting it when processing the get instruction works just as well. This change reduces our diff.
This reverts commit 60f3f25. We don't need to pass --disable-yjit when running MJIT tests anymore because we are off by default.
Previously, options such as "--yjit123" would enable YJIT. Additionally, the error message for argument parsing mentioned "--jit-..." instead of "--yjit-...".
When YJIT make calls to routines without reconstructing interpreter state through jit_prepare_routine_call(), it relies on the routine to never allocate, raise, and push/pop control frames. Comment about this on the routines that YJTI calls. This is probably something we should dynamically verify on debug builds. It's hard to statically verify this as it requires verifying all functions in the call tree. Maybe something to look at in the future.
Since conventionally scripts don't live at the top level of the repo.
DAE remember MicroJIT?
In an effort to simplify the logic YJIT generates for accessing instance variable, YJIT ensures that a given name-to-index mapping exists at compile time. In the case that the mapping doesn't exist, it was created by using rb_ivar_set() with Qundef on the sample object we see at compile time. This hack isn't fine if the sample object happens to be frozen, in which case YJIT would raise a FrozenError unexpectedly. To deal with this, make a new function that only reserves the mapping but doesn't touch the object. This is rb_obj_ensure_iv_index_mapping(). This new function superceeds the functionality of rb_iv_index_tbl_lookup() so it was removed. Reported by and includes a test case from John Hawthorn <john@hawthorn.email> Fixes: GH-282
So we don't try to run x64 on ARM.
Previously, we were shuffling keyword arguments before checking for interrupts. In the case that we side exit in the interrupt check, we left the interpreter with an already-shuffled argument list for the call, resulting in a double shuffle, leaving the locals in the wrong order for the callee. Do keyword shuffling after all the possible side exits. Co-authored-by: Kevin Newton <kddnewton@gmail.com>
519ce78
to
abf0532
Compare
On non RUBY_DEBUG builds, assert() compiles to nothing and the compiler warns about uninitialized variables in those code paths. Replace those asserts with rb_bug() to fix the warnings and do the assert in all builds. Since yjit_asm_tests.c compiles outside of Ruby, it needed a distinct version of rb_bug(). Also put YJIT_STATS check for function delcaration that is only defined in YJIT_STATS builds.
Fixes: ./src/yjit_asm.c:196:8: warning: 'mem_block' may be used uninitialized [-Wmaybe-uninitialized]
I merged it: 6a9e2b3 |
Understood. Sorry about the many commits 😅 |
BTW I tried Also Picked this from here - https://shopify.engineering/yjit-faster-rubying |
Hi @noahgibbs ! Would it be possible to update the blog post? |
Yup! I'll get on that. |
Hm. RUBY_YJIT_ENABLE is what I'm seeing in both the blog post and in the Ruby source code. I think we may have had it the other way some time back but changed it already? But what I'm seeing there now looks right, and I'm not finding YJIT_RUBY_ENABLE currently. @kapso, can you shift-reload the blog post in your browser and make sure you're seeing the latest version? |
This PR introduces YJIT, a just-in-time compiler built using a Lazy Basic Block Versioning (LBBV) compiler architecture. For more details about the technique, please refer to Maxime’s published paper and recorded talks:
YJIT currently provides average speedups of 23% over the CRuby interpreter on realistic benchmarks, and near-instant warm-up time. Plans are to include YJIT in the Ruby 3.1 preview release so that more users can test it out and benefit from potential performance gains.
Current limitations:
--yjit
to enable or setRUBY_YJIT_ENABLE=1
--yjit-exec-mem-size
is exceeded. This should be fixed in the coming months.--yjit-exec-mem-size
.YJIT integrates closely with the interpreter and some core runtime routines such as
rb_obj_is_kind_of()
andrb_str_eql_internal()
. YJIT also introduces a number of callbacks (seeyjit.h
) so that it can be notified of certain events, such as constants or methods being redefined. These changes are necessary so that YJIT can deoptimize code. These callbacks could potentially be useful for MJIT as well.YJIT does not spawn an external compiler as a sub-process for compilation. YJIT compiles concurrently with Ruby code execution on the same thread. Despite this, it provides near-instant warm-up. That is, YJIT typically delivers a speedup on the first iteration of our benchmarks.
Shopify is committed to making sure that YJIT remains in good working order. This PR adds new test workflows to help detect issues early on. Should bugs and issues arise, we will assist in investigating and fixing these in a timely manner. If necessary, code generation for specific YARV instructions can be disabled by commenting a single line near the end of
yjit_codegen.c
.We have tried to keep the YJIT code clean and approachable. The codebase is well-commented. We have a number of non-Shopify contributors who have found YJIT and submitted code.
YJIT makes additional runtime checks if compiled with
RUBY_DEBUG > 0
. It can also collect runtime statistics if compiled withRUBY_DEBUG
orYJIT_STATS > 0
and also run with the--yjit-stats
option or theRUBY_YJIT_STATS
environment variable set to 1.YJIT cannot be enabled at the same time as MJIT. Both can be compiled into Ruby, but only one can be active at runtime.
Contributors to the YJIT project so far:
Alan Wu
Aaron Patterson
Benson Muite
Dylan Smith
Eileen Uchitelle
Max Bernstein
Marc Feeley
Jose Narvaez
Jean Boussier
John Hawthorn
Kevin Newton
Maxime Chevalier-Boisvert
Mike Dalessio
Noah Gibbs
Takashi Kokubun
Ufuk Kayserilioglu