December 2023 - ruby-core - ml.ruby-lang.org

[ruby-core:115749] [Ruby master Feature#20066] Reduce Implicit Array/Hash Allocations For Method Calls Involving Splats
by jeremyevans0 (Jeremy Evans) 25 Jan '24

25 Jan '24

Issue #20066 has been reported by jeremyevans0 (Jeremy Evans). ---------------------------------------- Feature #20066: Reduce Implicit Array/Hash Allocations For Method Calls Involving Splats https://bugs.ruby-lang.org/issues/20066 * Author: jeremyevans0 (Jeremy Evans) * Status: Open * Priority: Normal ---------------------------------------- I have submitted a pull request (https://github.com/ruby/ruby/pull/9247) to reduce implicit array and hash allocations for method calls involving splats. The following optimizations are included: VM_CALL_ARGS_SPLAT_MUT callinfo flag This is similar to the VM_CALL_KW_SPLAT_MUT flag added in Ruby 3.0. This makes it so if the caller-side allocates an array for the method call, the flag is used to signal to the callee that it can reuse the allocated array and does not need to duplicate it. concattoarray VM instruction This instruction is similar to concatarray, but assumes the object being concatenated to is already a mutable array (such as those created by the splatarray VM instruction). This optimizes method calls with multiple splats such as `f(*a,*a,*a)` (which previously allocated 3 arrays), allocating a single array instead of an array per splatted array. pushtoarray VM instruction This is similar, but handles non-splat arguments after a splat. Previously, the VM would wrap those arguments in an array using newarray, and then call concatarray, such that `f(*a, a)` allocated 3 arrays caller-side. This instruction just appends to the mutable array, reducing the number of arrays allocated to 1. Allocationless Anonymous Splat Forwarding This allows `def f(*, **) end` to not allocate an array or hash callee side. This works because it is not possible to mutate the local variables, only pass them as splats to other methods. This can make the following call chain allocation less: ```ruby def f(a, b: 1) end def g(*, **) f(*, **) end ea a = [1] kw = {b: 2} g(*a, **kw) # No allocations in this call ``` Switch ... argument forwards to not use ruby2_keywords Using ruby2_keywords has probably been slower since Koichi's changes early in the Ruby 3.3 development cycle to not combine keyword splats into the positional splat array. This removes the FORWARD_ARGS_WITH_RUBY2_KEYWORDS define, so that `def f(...) end` operates similarly to `def f(*, **) end`, allowing allocationless splat forwarding Reduce array and hash allocations for nested argument forwarding calls This uses a combination of frame flags and callinfo flags to track mutability of anonymous splat variables. It can make it so the following call example only allocates a 1 array and 1 hash: ```ruby def m1(*args, **kw) end def m2(...) m1(...) end def m3(*, **) m2(*, **) end m3(1, a: 1) # 1 array and 1 hash allocated ``` In the above example, the call to `m3` allocates an array (`[1]`) and a hash (`{a: 1}`), but the call to `m2` passes them as mutable splats, `m2` treats them as mutable splats when calling `m1`, and `m1` reuses the array that `m3` allocated for `args` and the hash that `m3` allocated for `kw`. I created a benchmark for all of these changes. In the method calls optimized by these changes, it is significantly faster: ``` named_multi_arg_splat after: 5344097.6 i/s before: 3088134.0 i/s - 1.73x slower named_post_splat after: 5401882.3 i/s before: 2629321.8 i/s - 2.05x slower anon_arg_splat after: 12242780.9 i/s before: 6845413.2 i/s - 1.79x slower anon_arg_kw_splat after: 11277398.7 i/s before: 4329509.4 i/s - 2.60x slower anon_multi_arg_splat after: 5132699.5 i/s before: 3018103.7 i/s - 1.70x slower anon_post_splat after: 5602915.1 i/s before: 2645185.5 i/s - 2.12x slower anon_kw_splat after: 15403727.3 i/s before: 6249504.6 i/s - 2.46x slower anon_fw_to_named_splat after: 2985715.3 i/s before: 2049159.9 i/s - 1.46x slower anon_fw_to_named_no_splat after: 2941030.4 i/s before: 2100380.0 i/s - 1.40x slower fw_to_named_splat after: 2801008.7 i/s before: 2012416.4 i/s - 1.39x slower fw_to_named_no_splat after: 2742670.4 i/s before: 1957707.2 i/s - 1.40x slower fw_to_anon_to_named_splat after: 2309246.6 i/s before: 1375924.6 i/s - 1.68x slower fw_to_anon_to_named_no_splat after: 2193227.6 i/s before: 1351184.1 i/s - 1.62x slower ``` Only fallout from these changes: * Minor change to AST output for `...` not using `ruby2_keywords` * Prism and rbs need updating for `...` not using `ruby2_keywords` * typeprof need updating for new VM instructions (at least pushtoarray) VM_CALL_ARGS_SPLAT_MUT, concattoarray, and pushtoarray only affect uncommon callsites (multiple splats, argument after splat). Other commits only optimize calls to methods using anonymous splats or `...` argument forwarding. Previously, there was no performance reason to use anonymous splats or `...` argument forwarding, but with this change, using them can be faster, and can offer a new way for users to optimize their code. In my opinion, this is too late for consideration in Ruby 3.3, but it could be considered for Ruby 3.4. -- https://bugs.ruby-lang.org/

4 18

[ruby-core:114997] [Ruby master Bug#19918] Should `a[&b]=c` be syntax valid?
by tompng (tomoya ishida) 19 Jan '24

19 Jan '24

Issue #19918 has been reported by tompng (tomoya ishida). ---------------------------------------- Bug #19918: Should `a[&b]=c` be syntax valid? https://bugs.ruby-lang.org/issues/19918 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.3.0dev (2023-10-11T04:46:58Z master 40ab7b8c24) [x86_64-linux] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- These codes are syntax valid now. Prism parses it as syntax error. ~~~ruby a[&b]=c a[&b]+=c a[&b]&&=c a[&b]||=c ~~~ Is this syntax intentional or should be error? Issue of Prism https://github.com/ruby/prism/issues/1636 It's added in test_parse.rb https://github.com/ruby/ruby/blob/40ab7b8c244de20007cb45846f41de3a01f7ea0c/… -- https://bugs.ruby-lang.org/

5 6

[ruby-core:115594] [Ruby master Bug#20042] ObjectSpace finalizer can cause segfault
by ioquatix (Samuel Williams) 18 Jan '24

18 Jan '24

Issue #20042 has been reported by ioquatix (Samuel Williams). ---------------------------------------- Bug #20042: ObjectSpace finalizer can cause segfault https://bugs.ruby-lang.org/issues/20042 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- ```ruby class Finalizer def call # <- missing (id) argument end end object = Object.new ObjectSpace.define_finalizer(object, Finalizer.new) object = nil GC.start # segfaults here ``` -- https://bugs.ruby-lang.org/

3 3

[ruby-core:115830] [Ruby master Misc#20075] DevMeeting-2024-01-17
by mame (Yusuke Endoh) 17 Jan '24

17 Jan '24

Issue #20075 has been reported by mame (Yusuke Endoh). ---------------------------------------- Misc #20075: DevMeeting-2024-01-17 https://bugs.ruby-lang.org/issues/20075 * Author: mame (Yusuke Endoh) * Status: Open * Priority: Normal ---------------------------------------- # The next dev meeting **Date: 2024/01/17 13:00-17:00** (JST) Log: *TBD* - Dev meeting *IS NOT* a decision-making place. All decisions should be done at the bug tracker. - Dev meeting is a place we can ask Matz, nobu, nurse and other developers directly. - Matz is a very busy person. Take this opportunity to ask him. If you can not attend, other attendees can ask instead of you (if attendees can understand your issue). - We will write a record of the discussion in the file or to each ticket in English. - All activities are best-effort (keep in mind that most of us are volunteer developers). - The date, time and place of the meeting are scheduled according to when/where we can reserve Matz's time. - *DO NOT* discuss then on this ticket, please. # Call for agenda items If you have a ticket that you want matz and committers to discuss, please post it into this ticket in the following format: ``` * [Ticket ref] Ticket title (your name) * Comment (A summary of the ticket, why you put this ticket here, what point should be discussed, etc.) ``` Example: ``` * [Feature #14609] `Kernel#p` without args shows the receiver (ko1) * I feel this feature is very useful and some people say :+1: so let discuss this feature. ``` - It is recommended to add a comment by 2024/01/14. We hold a preparatory meeting to create an agenda a few days before the dev-meeting. - The format is strict. We'll use [this script to automatically create an markdown-style agenda](https://gist.github.com/mame/b0390509ce1491b43610b9ebb665eb86). We may ignore a comment that does not follow the format. - Your comment is mandatory. We cannot read all discussion of the ticket in a limited time. We appreciate it if you could write a short summary and update from a previous discussion. -- https://bugs.ruby-lang.org/

9 8

[ruby-core:111342] [Ruby master Bug#19246] Rebuilding the loaded feature index much slower in Ruby 3.1
by thomthom (Thomas Thomassen) 17 Jan '24

17 Jan '24

Issue #19246 has been reported by thomthom (Thomas Thomassen). ---------------------------------------- Bug #19246: Rebuilding the loaded feature index much slower in Ruby 3.1 https://bugs.ruby-lang.org/issues/19246 * Author: thomthom (Thomas Thomassen) * Status: Open * Priority: Normal * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- Some background to this issue: (This is a case that is unconventional usage of Ruby, but I hope you bear with me.) We ship the Ruby interpreter with our desktop applications for plugin support in our application (SketchUp). One feature we have had since, at least 2006 (maybe earlier-hard to track history beyond that) is that we had a custom alternate `require` method: `Sketchup.require`. This allows the users of our API to load encrypted Ruby files. This originally used `rb_provide` to add the path to the encrypted file into the list of loaded feature. However, somewhere between Ruby 2.2 and 2.5 there was some string optimisations made and the function `rb_provide` would not use a copy of the string passed to it. Instead it just held on to a pointer reference. In our case that string came from user-land, being passed in from `Sketchup.require` and would eventually be garbage collected and cause access violation crashes. To work around that we changed our custom `Sketchup.require` to push to `$LOADED_FEATURES` directly. There was a small penalty to the index being rebuilt after that, but it was negligible. Recently we tried to upgrade the Ruby interpreter in our application from 2.7 to 3.1 and found a major performance reduction when using our `Sketchup.require. As in, a plugin that would load in half a second would now spend 30 seconds. From https://bugs.ruby-lang.org/issues/18452 it sounds like there is _some_ expected extra penalty due to changes in how the index is built. But should it really be this much? Example minimal repro to simulate the issue: ``` # frozen_string_literal: true require 'benchmark' iterations = 200 foo_files = iterations.times.map { |i| "#{__dir__}/tmp/foo-#{i}.rb" } foo_files.each { |f| File.write(f, "") } bar_files = iterations.times.map { |i| "#{__dir__}/tmp/bar-#{i}.rb" } bar_files.each { |f| File.write(f, "") } biz_files = iterations.times.map { |i| "#{__dir__}/tmp/biz-#{i}.rb" } biz_files.each { |f| File.write(f, "") } Benchmark.bm do |x| x.report('normal') { foo_files.each { |file| require file } } x.report('loaded_features') { foo_files.each { |file| require file $LOADED_FEATURES << "#{file}-fake.rb" } } x.report('normal again') { biz_files.each { |file| require file } } end ``` ``` C:\Users\Thomas\SourceTree\ruby-perf>ruby27.bat ruby 2.7.4p191 (2021-07-07 revision a21a3b7d23) [x64-mingw32] C:\Users\Thomas\SourceTree\ruby-perf>ruby test-require.rb user system total real normal 0.000000 0.031000 0.031000 ( 0.078483) loaded_features 0.015000 0.000000 0.015000 ( 0.038759) normal again 0.016000 0.032000 0.048000 ( 0.076940) ``` ``` C:\Users\Thomas\SourceTree\ruby-perf>ruby30.bat ruby 2.7.4p191 (2021-07-07 revision a21a3b7d23) [x64-mingw32] C:\Users\Thomas\SourceTree\ruby-perf>ruby test-require.rb user system total real normal 0.000000 0.031000 0.031000 ( 0.074733) loaded_features 0.032000 0.000000 0.032000 ( 0.038898) normal again 0.000000 0.047000 0.047000 ( 0.076343) ``` ``` C:\Users\Thomas\SourceTree\ruby-perf>ruby31.bat ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x64-mingw-ucrt] C:\Users\Thomas\SourceTree\ruby-perf>ruby test-require.rb user system total real normal 0.016000 0.031000 0.047000 ( 0.132633) loaded_features 1.969000 11.500000 13.469000 ( 18.395761) normal again 0.031000 0.125000 0.156000 ( 0.249130) ``` Right now we're exploring options to deal with this. Because the performance degradation is a blocker for us upgrading. We also have 16 years of plugins created by third party developer that makes it impossible for us to drop this feature. Some options as-is, none of which are ideal: 1. We revert to using `rb_provide` but ensure the string passed in is not owned by Ruby, instead building a list of strings that we keep around for the duration of the application process. The problem is that some of our plugin developers have on occasion released plugins that will touch `$LOADED_FEATURES`, and if such a plugin is installed on a user machine it might cause the application to become unresponsive for minutes. The other non-ideal issue with using `rb_provide` is that we're also using that in ways it wasn't really intended (from that I understand). And it's not an official API? 2. We create a separate way for our `Sketchup.require` to keep track of it's loaded features, but then that would diverge even more from the behaviour of `require`. Replicating `require` functionality is not trivial and would be prone to subtle errors and possible diverge. It also doesn't address our issue that there is code out there in existing plugins that touches `$LOADED_FEATURES`. (And it's not something we can just ask people to clean up. From previous experience old versions stick around for a long time and is very hard to purge from circulation.) I have two questions for the Ruby mantainers: 1. Would it be reasonable to see an API for adding/removing/checking `$LOADED_FEATURE` that would allow for a more ideal implementation of a custom `require` functionality? 2. Is the performance difference in rebuilding the loaded feature index really expected to be as high as what we're seeing? An increase of nearly 100 times? Is there something there that might be addressed to make the rebuild to be less expensive against? (This would really help to address our challenges with third party plugins occasionally touching the global.) -- https://bugs.ruby-lang.org/

9 21

[ruby-core:115181] [Ruby master Bug#19975] ISeq#to_binary loses hidden local variable indices
by kddnewton (Kevin Newton) 12 Jan '24

12 Jan '24

Issue #19975 has been reported by kddnewton (Kevin Newton). ---------------------------------------- Bug #19975: ISeq#to_binary loses hidden local variable indices https://bugs.ruby-lang.org/issues/19975 * Author: kddnewton (Kevin Newton) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- When you call `RubyVM::InstructionSequence#to_a`, you get hidden local variables as the index in the stack from the top: ``` c if (rb_id2str(lid)) { rb_ary_push(locals, ID2SYM(lid)); } else { /* hidden variable from id_internal() */ rb_ary_push(locals, ULONG2NUM(iseq_body->local_table_size-i+1)); } ``` ``` rb RubyVM::InstructionSequence.compile("for foo in bar; end").to_a[13][4][2][10] # => [2] ``` When you call `RubyVM::InstructionSequence#to_binary`, it dumps hidden local variables as 0: ``` c if (id == 0 || rb_id2name(id) == NULL) { return 0; } return ibf_dump_object(dump, rb_id2sym(id)); ``` When it reads that back in and then you call `to_a`, you get `:#arg_rest`: ``` ruby RubyVM::InstructionSequence.load_from_binary(RubyVM::InstructionSequence.compile("for foo in bar; end").to_binary).to_a[13][4][2][10] # => [:"#arg_rest"] ``` This means you end up not being able to consistently look at the locals. Instead, when reading back in from binary it could replace it with the index so that it matches up with the value before it is dumped to binary. Could we do that so that `#to_a` is consistent no matter where the iseq came from? -- https://bugs.ruby-lang.org/

2 1

[ruby-core:115212] [Ruby master Bug#19983] Nested * seems incorrect
by Eregon (Benoit Daloze) 11 Jan '24

11 Jan '24

Issue #19983 has been reported by Eregon (Benoit Daloze). ---------------------------------------- Bug #19983: Nested * seems incorrect https://bugs.ruby-lang.org/issues/19983 * Author: Eregon (Benoit Daloze) * Status: Open * Priority: Normal * ruby -v: ruby 3.3.0dev (2023-10-30T09:27:06Z master 14fa5e39d7) [x86_64-linux] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- ``` $ ruby -v -e 'def m(*); ->(*) { p(*) }; end; m(1).call(2)' ruby 3.3.0dev (2023-10-30T09:27:06Z master 14fa5e39d7) [x86_64-linux] 1 ``` But I would expect `2`. Much like: ``` $ ruby -e 'def m(a); ->(a) { p(a) }; end; m(1).call(2)' ruby 3.3.0dev (2023-10-30T09:27:06Z master 14fa5e39d7) [x86_64-linux] 2 ``` i.e. the inner variable should win. -- https://bugs.ruby-lang.org/

4 8

[ruby-core:115845] [Ruby master Bug#20078] StringIO cannot be moved between Ractors
by forthoney (Seong-Heon Jung) 11 Jan '24

11 Jan '24

Issue #20078 has been reported by forthoney (Seong-Heon Jung). ---------------------------------------- Bug #20078: StringIO cannot be moved between Ractors https://bugs.ruby-lang.org/issues/20078 * Author: forthoney (Seong-Heon Jung) * Status: Open * Priority: Normal * ruby -v: 3.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- The following code will raise an unusual Ractor error. ``` ruby require 'stringio' r = Ractor.new { loop { Ractor.receive } } Ractor.shareable?(StringIO.new) #=> false r.send(StringIO.new) # passing it via copy works r.send(StringIO.new, move: true) # <internal:ractor>:587:in `send': can not move StringIO object. (Ractor::Error) ``` I'm not 100% sure but I believe that this probably not the intended behavior considering * The error raised is a generic `Ractor::Error` rather than something specific like Ractor::MovedError or Ractor::IsolationError * It can be copied * [No documentation](https://docs.ruby-lang.org/en/master/ractor_md.html) exists for this error * I have yet to seen this happen on any other class instance * Typo (can not -> cannot) -- https://bugs.ruby-lang.org/

3 6

[ruby-core:115736] [Ruby master Bug#20064] Inconsistent behavior between array splat *nil and hash splat **nil
by zeke (Zeke Gabrielse) 11 Jan '24

11 Jan '24

Issue #20064 has been reported by zeke (Zeke Gabrielse). ---------------------------------------- Bug #20064: Inconsistent behavior between array splat *nil and hash splat **nil https://bugs.ruby-lang.org/issues/20064 * Author: zeke (Zeke Gabrielse) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- This has been discussed in #8507 and #9291 but both were closed because they lacked a clear use case. I think the following code should work, showing a clear use case: ```ruby invitation = if params.key?(:inviter_id) { invitation_attributes: params.slice(:inviter_id) } end User.create( email: 'john.doe(a)ruby.example', first_name: 'John', first_name: 'Doe', **invitation, ) ``` Per the previous discussions, this is because `*` uses explicit conversion to array (`to_a`, not `to_ary`), while `**` uses implicit conversion to hash (`to_hash`, not `to_h`). I find it confusing that you can splat nil into an array, but not nil into a hash. It would make sense for `**` to use explicit conversion. -- https://bugs.ruby-lang.org/

4 4

[ruby-core:115491] [Ruby master Feature#20024] SyntaxError subclasses
by kddnewton (Kevin Newton) 10 Jan '24

10 Jan '24

Issue #20024 has been reported by kddnewton (Kevin Newton). ---------------------------------------- Feature #20024: SyntaxError subclasses https://bugs.ruby-lang.org/issues/20024 * Author: kddnewton (Kevin Newton) * Status: Open * Priority: Normal ---------------------------------------- There are many places around the Ruby ecosystem that handle syntax errors in different ways. Some provide highlighting, others provide recovery of some form, still more provide LSP metadata. In order to provide more rich information, most of them switch on the message of the error being returned, as in: https://github.com/ruby/irb/blob/f86d9dbe2fc05ed62332069a27f4aacc59ba9634/l… Within ruby/spec, specific error messages are required for these kinds of messages in order to support this implicit interface that syntax errors have a hidden type, which is only expressed through their message. For example: https://github.com/ruby/spec/blob/c3206f644325c026fc5b700f0ea75ce9bd2e9d02/… https://github.com/ruby/spec/blob/c3206f644325c026fc5b700f0ea75ce9bd2e9d02/… https://github.com/ruby/spec/blob/c3206f644325c026fc5b700f0ea75ce9bd2e9d02/… https://github.com/ruby/spec/blob/c3206f644325c026fc5b700f0ea75ce9bd2e9d02/… https://github.com/ruby/spec/blob/c3206f644325c026fc5b700f0ea75ce9bd2e9d02/… It's not clear from these specs or from the parser itself which error messages are permanent/guaranteed versus which are changeable. Either way, relying on the error message itself as opposed to the type of the error is brittle at best. I would like to suggest instead we implement subclasses on `SyntaxError` that would allow tools that depend on specific syntax errors to rescue those subclasses instead of parsing the message. In addition to alleviating the need to parse error messages with regex, this would also allow for the possibility that the error messages could change in the future without breaking external tooling. Allowing these to change would allow them to be potentially enhanced or changed by other tools - for example by providing recovery information or translating them. This is particularly important for Prism since we are getting down to individual spec failures and some of the failures are related to the fact that we have messages like `"Numbered parameter is already used in outer scope"` where the spec requires `/numbered parameter is already used in/`. Even this case-sensitivity is causing failures, which seems like we're testing the wrong thing. -- https://bugs.ruby-lang.org/

9 16