Bug #20473
open
Ractor array/hash literals
Description
There are many optimizations in place in the compiler to ensure Ruby doesn't overflow the stack when an array or hash literal has many values. For example:
puts RubyVM::InstructionSequence.compile(<<~RUBY).disasm
[#{Array.new(512) { |index| index % 128 == 0 ? "\"\#{nil}\"" : "1" }.join(", ")}]
RUBY
gives the instructions:
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,1564)>
0000 putobject "" ( 1)[Li]
0002 putnil
0003 dup
0004 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0006 anytostring
0007 concatstrings 2
0009 newarray 1
0011 putobject [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
0013 concatarray
0014 putobject ""
0016 putnil
0017 dup
0018 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0020 anytostring
0021 concatstrings 2
0023 newarray 1
0025 concatarray
0026 putobject [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
0028 concatarray
0029 putobject ""
0031 putnil
0032 dup
0033 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0035 anytostring
0036 concatstrings 2
0038 newarray 1
0040 concatarray
0041 putobject [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
0043 concatarray
0044 putobject ""
0046 putnil
0047 dup
0048 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0050 anytostring
0051 concatstrings 2
0053 newarray 1
0055 concatarray
0056 putobject [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
0058 concatarray
0059 leave
because it is smart enough to group the static elements together and use concatarray. This ensures we never have too many values on the stack.
In Ruby 3.3, this worked for ractors as well. (Adding # shareable_constant_value: literal and A = to the example above doesn't change anything about the instructions above except adding a couple of -@ calls and one call to make_shareable.
On HEAD, all of these optimizations are now gone. With this:
puts RubyVM::InstructionSequence.compile(<<~RUBY).disasm
# shareable_constant_value: literal
A = [#{Array.new(512) { |index| index % 128 == 0 ? "\"\#{nil}\"" : "1" }.join(", ")}]
RUBY
code on master, we get:
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(2,1568)>
0000 putobject RubyVM::FrozenCore ( 2)[Li]
0002 putobject ""
0004 putnil
0005 dup
0006 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0008 anytostring
0009 concatstrings 2
0011 opt_send_without_block <calldata!mid:-@, argc:0, ARGS_SIMPLE>
0013 putobject_INT2FIX_1_
~~~~
0139 putobject_INT2FIX_1_
0140 putobject ""
0142 putnil
0143 dup
0144 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0146 anytostring
0147 concatstrings 2
0149 opt_send_without_block <calldata!mid:-@, argc:0, ARGS_SIMPLE>
0151 putobject_INT2FIX_1_
~~~~
0277 putobject_INT2FIX_1_
0278 putobject ""
0280 putnil
0281 dup
0282 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0284 anytostring
0285 concatstrings 2
0287 opt_send_without_block <calldata!mid:-@, argc:0, ARGS_SIMPLE>
0289 putobject_INT2FIX_1_
~~~~
0415 putobject_INT2FIX_1_
0416 putobject ""
0418 putnil
0419 dup
0420 objtostring <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0422 anytostring
0423 concatstrings 2
0425 opt_send_without_block <calldata!mid:-@, argc:0, ARGS_SIMPLE>
0427 putobject_INT2FIX_1_
~~~~
0553 putobject_INT2FIX_1_
0554 newarray 512
0556 opt_send_without_block <calldata!mid:make_shareable, argc:1, ARGS_SIMPLE>
0558 dup
0559 putspecialobject 3
0561 setconstant :A
0563 leave
where the ~~~~ represents another 128 putobject_INT2FIX_1_ instructions.
I assume this was done because it used to modify the AST and that got removed. However, I also assume it was not intentional to lose all of these optimizations, as it now means if Ractors are being used that large literals are vulnerable to stack overflows.
Please let me know how to proceed, as I am trying to exactly match behavior in the prism compiler.
Updated by luke-gru (Luke Gruber) 2 months ago
I added back the array literal optimizations here: https://github.com/luke-gru/ruby/tree/bug20473
I didn't send a PR because it's not working with prism yet. Let me know if this is an okay solution, I could get it working for prism too.
I haven't worked on the hash optimizations but it should be similar.
Updated by kddnewton (Kevin Newton) 2 months ago
Truthfully it might not be worth adding these optimizations. It's a lot to maintain in both compilers, and this kind of literal is extremely unlikely to be used with Ractors until other problems with Ractors are solved. I would just as soon not have to maintain this part of the compiler.
Another option would be to put this into the peephole optimizer, which would make it so that you didn't have to update both compilers.
Updated by luke-gru (Luke Gruber) 2 months ago
Okay, makes sense. Maybe if/when ractors are used more I'll take a look at this again.