After [1], using ext/Setup to link some, but not all extensions failed
during linking. I did not know about this option, and had assumed that
only `--with-static-linked-ext` builds can include statically linked
extensions.
Include the support code for statically linked extensions in all
configurations like before [1]. Initialize the table lazily to minimize
footprint on builds that have no statically linked extensions.
[1]: 790cf4b6d0 "Fix autoload status of
statically linked extensions"
Originally, when 2e7bceb34e fixed cfuncs to no
longer use the VM stack for large array splats, it was thought to have fully
fixed Bug #4040, since the issue was fixed for methods defined in Ruby (iseqs)
back in Ruby 2.2.
After additional research, I determined that same issue affects almost all
types of method calls, not just iseq and cfunc calls. There were two main
types of remaining issues, important cases (where large array splat should
work) and pedantic cases (where large array splat raised SystemStackError
instead of ArgumentError).
Important cases:
```ruby
define_method(:a){|*a|}
a(*1380888.times)
def b(*a); end
send(:b, *1380888.times)
:b.to_proc.call(self, *1380888.times)
def d; yield(*1380888.times) end
d(&method(:b))
def self.method_missing(*a); end
not_a_method(*1380888.times)
```
Pedantic cases:
```ruby
def a; end
a(*1380888.times)
def b(_); end
b(*1380888.times)
def c(_=nil); end
c(*1380888.times)
c = Class.new do
attr_accessor :a
alias b a=
end.new
c.a(*1380888.times)
c.b(*1380888.times)
c = Struct.new(:a) do
alias b a=
end.new
c.a(*1380888.times)
c.b(*1380888.times)
```
This patch fixes all usage of CALLER_SETUP_ARG with splatting a large
number of arguments, and required similar fixes to use a temporary
hidden array in three other cases where the VM would use the VM stack
for handling a large number of arguments. However, it is possible
there may be additional cases where splatting a large number
of arguments still causes a SystemStackError.
This has a measurable performance impact, as it requires additional
checks for a large number of arguments in many additional cases.
This change is fairly invasive, as there were many different VM
functions that needed to be modified to support this. To avoid
too much API change, I modified struct rb_calling_info to add a
heap_argv member for storing the array, so I would not have to
thread it through many functions. This struct is always stack
allocated, which helps ensure sure GC doesn't collect it early.
Because of how invasive the changes are, and how rarely large
arrays are actually splatted in Ruby code, the existing test/spec
suites are not great at testing for correct behavior. To try to
find and fix all issues, I tested this in CI with
VM_ARGC_STACK_MAX to -1, ensuring that a temporary array is used
for all array splat method calls. This was very helpful in
finding breaking cases, especially ones involving flagged keyword
hashes.
Fixes [Bug #4040]
Co-authored-by: Jimmy Miller <jimmy.miller@shopify.com>
This commit introduces a new instruction `opt_newarray_send` which is
used when there is an array literal followed by either the `hash`,
`min`, or `max` method.
```
[a, b, c].hash
```
Will emit an `opt_newarray_send` instruction. This instruction falls
back to a method call if the "interested" method has been monkey
patched.
Here are some examples of the instructions generated:
```
$ ./miniruby --dump=insns -e '[@a, @b].max'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
0000 getinstancevariable :@a, <is:0> ( 1)[Li]
0003 getinstancevariable :@b, <is:1>
0006 opt_newarray_send 2, :max
0009 leave
$ ./miniruby --dump=insns -e '[@a, @b].min'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
0000 getinstancevariable :@a, <is:0> ( 1)[Li]
0003 getinstancevariable :@b, <is:1>
0006 opt_newarray_send 2, :min
0009 leave
$ ./miniruby --dump=insns -e '[@a, @b].hash'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,13)> (catch: FALSE)
0000 getinstancevariable :@a, <is:0> ( 1)[Li]
0003 getinstancevariable :@b, <is:1>
0006 opt_newarray_send 2, :hash
0009 leave
```
[Feature #18897] [ruby-core:109147]
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Rebuilding the loaded feature index slowed down with the bug fix
for #17885 in 79a4484a07. The
slowdown was extreme if realpath emulation was used, but even when
not emulated, it could be about 10x slower.
This adds loaded_features_realpath_map to rb_vm_struct. This is a
hidden hash mapping loaded feature paths to realpaths. When
rebuilding the loaded feature index, look at this hash to get
cached realpath values, and skip calling rb_check_realpath if a
cached value is found.
Fixes [Bug #19246]
Show native thread's serial on `RUBY_DEBUG_LOG`.
`nt->serial` is also stored into `ruby_nt_serial` if the compiler
supports `RB_THREAD_LOCAL_SPECIFIER`.
C function frames don't need to use the VM-specific pc field to run
properly. When pushing a control frame from output code, save one
instruction by leaving the field uninitialized.
Fix-up rb_vm_svar_lep(), which is used while setting local variables via
Regexp#=~. Use cfp->iseq as a secondary signal so it can stop assuming
that all CFUNC frames always have zero pc's.
* YJIT: Add --yjit-pause and RubyVM::YJIT.resume
This allows booting YJIT in a suspended state. We chose to add a new
command line option as opposed to simply allowing YJIT.resume to work
without any command line option because it allows for combining with
YJIT tuning command line options. It also simpifies implementation.
Paired with Kokubun and Maxime.
* Update yjit.rb
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
---------
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
We can only allocate enough shapes to fit in the shape buffer.
MAX_SHAPE_ID was based on the theoretical maximum number of shapes we
could have, not on the amount of memory we can actually consume. This
commit changes the MAX_SHAPE_ID to be based on the amount of memory
we're allowed to consume.
Co-Authored-By: Jemma Issroff <jemmaissroff@gmail.com>
* Revert "Remove special handling of `SIGCHLD`. (#7482)"
This reverts commit 44a0711eab.
* Revert "Remove prototypes for functions that are no longer used. (#7497)"
This reverts commit 4dce12bead.
* Revert "Remove SIGCHLD `waidpid`. (#7476)"
This reverts commit 1658e7d966.
* Fix change to rjit variable name.
We used to require MJIT is supported when YJIT is supported. However,
now that RJIT dropped some platforms that YJIT supports, it no longer
makes sense. We should be able to enable only YJIT, and vice versa.
Right now the attached object is stored as an instance variable
and all the call sites that either get or set it have to know how it's
stored.
It's preferable to hide this implementation detail behind accessors
so that it is easier to change how it's stored.
This patch is follo-up of 0a82bfe.
Without this patch, if env is escaped (Proc'ed), strange svar
can be touched.
This patch tracks escaped env and use it.
* use correct svar
Without this patch, svar location is used "nearest Ruby frame".
It is almost correct but it doesn't correct when the `each` method
is written in Ruby.
```ruby
class C
include Enumerable
def each
%w(bar baz).each{|e| yield e}
end
end
C.new.grep(/(b.)/){|e| p [$1, e]}
```
This patch fix this issue by traversing ifunc's cfp.
Note that if cfp doesn't specify this Thread's cfp stack, reserved
svar location (`ec->root_svar`) is used.
* make yjit-bindgen
---------
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Applying the following patch to test/erb/test_erb.rb and running that
file will cause Ruby to crash on my machine (macOS 13.1 on M1 Pro):
```
--- a/test/erb/test_erb.rb
+++ b/test/erb/test_erb.rb
@@ -7,6 +7,12 @@
class TestERB < Test::Unit::TestCase
class MyError < RuntimeError ; end
+ def setup
+ GC.auto_compact = true
+ GC.stress = true
+ GC.verify_compaction_references(expand_heap: true, toward: :empty)
+ end
+
```
It crashes with the following log:
```
/Users/peter/src/ruby/lib/erb/compiler.rb:276: [BUG] Segmentation fault at 0x00000001083a8690
...
-- C level backtrace information -------------------------------------------
...
/Users/peter/src/ruby/build/ruby(rb_vm_each_stack_value+0xa8) [0x104cc3a44] ../vm.c:2737
/Users/peter/src/ruby/build/ruby(rb_vm_each_stack_value+0xa8) [0x104cc3a44] ../vm.c:2737
/Users/peter/src/ruby/build/ruby(check_stack_for_moved+0x2c) [0x104b272a4] ../gc.c:5512
/Users/peter/src/ruby/build/ruby(gc_compact_finish) ../gc.c:5534
/Users/peter/src/ruby/build/ruby(gc_sweep_compact) ../gc.c:8653
/Users/peter/src/ruby/build/ruby(gc_sweep) ../gc.c:6196
/Users/peter/src/ruby/build/ruby(has_sweeping_pages+0x0) [0x104b19c54] ../gc.c:9568
/Users/peter/src/ruby/build/ruby(gc_rest) ../gc.c:9570
```
This crash happens because it's reading the VALUE at sp. But since
sp points to the top of the stack, it's reading the VALUE above the
top of the stack, which is causing this segfault.
Fixes [Bug #19320]
I noticed this while running test_yjit with --mjit-call-threshold=1,
which redefines `Integer#<`. When Ruby is monkey-patched,
MJIT itself could be broken.
Similarly, Ruby scripts could break MJIT in many different ways. I
prepared the same set of hooks as YJIT so that we could possibly
override it and disable it on those moments. Every constant under
RubyVM::MJIT is private and thus it's an unsupported behavior though.
On a hash miss we need to call default if it is redefined in order to
return the default value to be used. Previously we checked this with
rb_method_basic_definition_p, which avoids the method call but requires
a method lookup.
This commit replaces the previous check with BASIC_OP_UNREDEFINED_P and
a new BOP_DEFAULT. We still need to fall back to
rb_method_basic_definition_p when called on a subclasss of hash.
| |compare-ruby|built-ruby|
|:---------------|-----------:|---------:|
|hash_aref_miss | 2.692| 3.531|
| | -| 1.31x|
Co-authored-by: Daniel Colson <danieljamescolson@gmail.com>
Co-authored-by: "Ian C. Anderson" <ian@iancanderson.com>
Co-authored-by: Jack McCracken <me@jackmc.xyz>
All values should have a MJIT_ prefix. We could address the warning for
the end mark if we just define the macro for the check next to the enum.
It even simplifies some code for checking the enum.
Prior to this commit the `OPTIMIZED_CMP` macro relied on a method lookup
to determine whether `<=>` was overridden. The result of the lookup was
cached, but only for the duration of the specific method that
initialized the cmp_opt_data cache structure.
With this method lookup, `[x,y].max` is slower than doing `x > y ?
x : y` even though there's an optimized instruction for "new array max".
(John noticed somebody a proposed micro-optimization based on this fact
in https://github.com/mastodon/mastodon/pull/19903.)
```rb
a, b = 1, 2
Benchmark.ips do |bm|
bm.report('conditional') { a > b ? a : b }
bm.report('method') { [a, b].max }
bm.compare!
end
```
Before:
```
Comparison:
conditional: 22603733.2 i/s
method: 19820412.7 i/s - 1.14x (± 0.00) slower
```
This commit replaces the method lookup with a new CMP basic op, which
gives the examples above equivalent performance.
After:
```
Comparison:
method: 24022466.5 i/s
conditional: 23851094.2 i/s - same-ish: difference falls within
error
```
Relevant benchmarks show an improvement to Array#max and Array#min when
not using the optimized newarray_max instruction as well. They are
noticeably faster for small arrays with the relevant types, and the same
or maybe a touch faster on larger arrays.
```
$ make benchmark COMPARE_RUBY=<master@5958c305> ITEM=array_min
$ make benchmark COMPARE_RUBY=<master@5958c305> ITEM=array_max
```
The benchmarks added in this commit also look generally improved.
Co-authored-by: John Hawthorn <jhawthorn@github.com>
This commit moves ruby_basic_operators and the unredefined macros out of
vm_core.h and into basic_operators.h so that we can use them more
broadly in places where we currently use a method look up via
`rb_method_basic_definition_p` (e.g. object.c, numeric.c, complex.c,
enum.c, but also in internal/compar.h after introducing BOP_CMP and
elsewhere if we introduce more BOPs)
The most controversial part of this change is probably moving
redefined_flag out of rb_vm_t. [vm_opt_method_def_table and
vm_opt_mid_table](9da2a5204f/vm.c)
are not part of rb_vm_t either, and I think this fits well with those.
But more significantly it seems to result in one fewer instruction. For
example:
Before:
```
(lldb) disassemble -n vm_opt_str_freeze
miniruby`vm_exec_core:
miniruby[0x10028233e] <+14558>: movq 0x11a86b(%rip), %rax ; ruby_current_vm_ptr
miniruby[0x100282345] <+14565>: testb $0x4, 0x242c(%rax)
```
After:
```
(lldb) disassemble -n vm_opt_str_freeze
ruby`vm_exec_core:
ruby[0x100280ebe] <+14510>: testb $0x4, 0x120147(%rip) ; ruby_vm_redefined_flag + 43
```
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Previously, for statically-linked extensions, we used
`vm->loading_table` to delay calling the init function until the
extensions are required. This caused the extensions to look like they
are in the middle of being loaded even before they're required.
(`rb_feature_p()` returned true with a loading path output.) Combined
with autoload, queries like `defined?(CONST)` and `Module#autoload?`
were confused by this and returned nil incorrectly. RubyGems uses
`defined?` to detect if OpenSSL is available and failed when OpenSSL was
available in builds using `--with-static-linked-ext`.
Use a dedicated table for the init functions instead of adding them to
the loading table. This lets us remove some logic from non-EXTSTATIC
builds.
[Bug #19115]
We need to track this number in CI. It's important to know how changes
to the codebase impact the number of shapes in the system, so lets add
the number to the VM stat hash
The structure and readability of jit_exec is messed up right now. I'd
like to help the current situation by this for now. I'll resurrect
them when I need it again.
This commit adds a `capacity` field to shapes, and adds shape
transitions whenever an object's capacity changes. Objects which are
allocated out of a bigger size pool will also make a transition from the
root shape to the shape with the correct capacity for their size pool
when they are allocated.
This commit will allow us to remove numiv from objects completely, and
will also mean we can guarantee that if two objects share shapes, their
IVs are in the same positions (an embedded and extended object cannot
share shapes). This will enable us to implement ivar sets in YJIT using
object shapes.
Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
