Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Nov 15, 2021

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

ko1 and others added 14 commits November 15, 2021 13:11
This function is used in `rb_vm_lvar()` and this function can be
unsed (generated into *.rbinc files automatically).
`rb_vm_lvar()` is already declared as a PUREFUNC, but
`rb_vm_lvar_exposed()` is not a PUREFUNC, so `rb_vm_lvar_exposed()`
is remained even if it is unused.
The test uses `Encoding.default_external = Encoding::UTF_16BE`, which
may add a wrongly UTF_16BE-encoded path to $LOADED_FEATURES (depending
on the order of tests). Unfortunately this breaks another test:

http://ci.rvm.jp/results/trunk-test@ruby-sky1/3711615
```
/tmp/ruby/v3/src/trunk-test/test/io/console/test_io_console.rb:11:in `===': incompatible encoding regexp match (US-ASCII regexp with UTF-16BE string) (Encoding::CompatibilityError)
```

According to @naruse-san, we don't pay effort to such a case, so this
change just avoids the issue by running the test in question under
another process.

Co-Authored-By: Koichi Sasada <ko1@atdot.net>
This reverts commit fc456ad.

It broke the CI check.

https://github.com/ruby/ruby/runs/4207922247?check_suite_focus=true#step:3:4
```
numeric.c:3518: *     255.chr(Encoding::UTF_8) # => "ÿ"
Error: Process completed with exit code 1.
```
This reverts commit a698181.

It failed on macos for a unknown problem.
```
    1) Error:
  TestM17N#test_object_inspect_external:
  Encoding::ConverterNotFoundError: code converter not found (US-ASCII to UTF-16BE)
      /Users/runner/work/ruby/ruby/src/test/ruby/test_m17n.rb:312:in `encode'
      /Users/runner/work/ruby/ruby/src/test/ruby/test_m17n.rb:312:in `inspect'
      /Users/runner/work/ruby/ruby/src/test/ruby/test_m17n.rb:315:in `inspect'
      /Users/runner/work/ruby/ruby/src/test/ruby/test_m17n.rb:315:in `<main>'
      /Users/runner/work/ruby/ruby/src/test/ruby/test_m17n.rb:299:in `test_object_inspect_external'
```
https://github.com/ruby/ruby/runs/4207871418?check_suite_focus=true
Compare with the C methods, A built-in methods written in Ruby is
slower if only mandatory parameters are given because it needs to
check the argumens and fill default values for optional and keyword
parameters (C methods can check the number of parameters with `argc`,
so there are no overhead). Passing mandatory arguments are common
(optional arguments are exceptional, in many cases) so it is important
to provide the fast path for such common cases.

`Primitive.mandatory_only?` is a special builtin function used with
`if` expression like that:

```ruby
  def self.at(time, subsec = false, unit = :microsecond, in: nil)
    if Primitive.mandatory_only?
      Primitive.time_s_at1(time)
    else
      Primitive.time_s_at(time, subsec, unit, Primitive.arg!(:in))
    end
  end
```

and it makes two ISeq,

```
  def self.at(time, subsec = false, unit = :microsecond, in: nil)
    Primitive.time_s_at(time, subsec, unit, Primitive.arg!(:in))
  end

  def self.at(time)
    Primitive.time_s_at1(time)
  end
```

and (2) is pointed by (1). Note that `Primitive.mandatory_only?`
should be used only in a condition of an `if` statement and the
`if` statement should be equal to the methdo body (you can not
put any expression before and after the `if` statement).

A method entry with `mandatory_only?` (`Time.at` on the above case)
is marked as `iseq_overload`. When the method will be dispatch only
with mandatory arguments (`Time.at(0)` for example), make another
method entry with ISeq (2) as mandatory only method entry and it
will be cached in an inline method cache.

The idea is similar discussed in https://bugs.ruby-lang.org/issues/16254
but it only checks mandatory parameters or more, because many cases
only mandatory parameters are given. If we find other cases (optional
or keyword parameters are used frequently and it hurts performance),
we can extend the feature.
```
                      ruby_2_6    ruby_2_7    ruby_3_0      master    modified
         ary.sample    32.113M     30.146M     11.162M     10.539M     26.620M i/s -     64.882M times in 2.020428s 2.152296s 5.812981s 6.156398s 2.437325s
      ary.sample(2)     9.420M      8.987M      7.500M      6.973M      7.191M i/s -     25.170M times in 2.672085s 2.800616s 3.355896s 3.609534s 3.500108s
```

```
ruby_2_6: ruby 2.6.7p150 (2020-12-09 revision 67888) [x86_64-linux]
ruby_2_7: ruby 2.7.3p140 (2020-12-09 revision 9b884df) [x86_64-linux]
ruby_3_0: ruby 3.0.3p150 (2021-11-06 revision 6d540c1) [x86_64-linux]
master: ruby 3.1.0dev (2021-11-13T20:48:57Z master fc456ad) [x86_64-linux]
modified: ruby 3.1.0dev (2021-11-15T01:12:51Z mandatory_only_bui.. b0228446db) [x86_64-linux]
```
```
                                ruby_2_6    ruby_2_7    ruby_3_0      master    modified
                   Time.at(0)    12.362M     11.015M      9.499M      6.615M      9.000M i/s -     32.115M times in 2.597946s 2.915517s 3.380725s 4.854651s 3.568234s
              Time.at(0, 500)     7.542M      7.136M      8.252M      5.707M      5.646M i/s -     20.713M times in 2.746279s 2.902556s 2.510166s 3.629644s 3.668854s
     Time.at(0, in: "+09:00")     1.426M      1.346M      1.565M      1.674M      1.667M i/s -      4.240M times in 2.974049s 3.149753s 2.709416s 2.533043s 2.542853s
```

```
ruby_2_6: ruby 2.6.7p150 (2020-12-09 revision 67888) [x86_64-linux]
ruby_2_7: ruby 2.7.3p140 (2020-12-09 revision 9b884df) [x86_64-linux]
ruby_3_0: ruby 3.0.3p150 (2021-11-06 revision 6d540c1) [x86_64-linux]
master: ruby 3.1.0dev (2021-11-13T20:48:57Z master fc456ad) [x86_64-linux]
modified: ruby 3.1.0dev (2021-11-15T01:12:51Z mandatory_only_bui.. b0228446db) [x86_64-linux]
```
and also drop a weird newline from benchmark/array_sample.yml.
@pull pull bot added the ⤵️ pull label Nov 15, 2021
@devkadirselcuk devkadirselcuk merged commit e103966 into turkdevops:master Nov 16, 2021
pull bot pushed a commit that referenced this pull request Sep 6, 2023
[Bug #19793]

Dummy frames are created at the top level when requiring another file.
While requiring a file, it will try to convert using encodings. Some of
these encodings will not respond to to_str. If method_missing is
redefined on Object, then it will call method_missing and attempt raise
an error. However, the iseq is invalid as it's a dummy frame so it will
write an invalid iseq to the created NoMethodError.

The following script crashes:

```
GC.stress = true

class Object
  public :method_missing
end

File.write("/tmp/empty.rb", "")
require "/tmp/empty.rb"
```

With the following backtrace:

```
frame #0: 0x00000001000fa8b8 miniruby`RVALUE_MARKED(obj=4308637824) at gc.c:1638:12
frame #1: 0x00000001000fb440 miniruby`RVALUE_BLACK_P(obj=4308637824) at gc.c:1763:12
frame #2: 0x00000001000facdc miniruby`gc_writebarrier_incremental(a=4308637824, b=4308332208, objspace=0x000000010180b000) at gc.c:8822:9
frame #3: 0x00000001000faad8 miniruby`rb_gc_writebarrier(a=4308637824, b=4308332208) at gc.c:8864:17
frame #4: 0x000000010016aff0 miniruby`rb_obj_written(a=4308637824, oldv=36, b=4308332208, filename="../iseq.c", line=1279) at gc.h:804:9
frame #5: 0x0000000100162a60 miniruby`rb_obj_write(a=4308637824, slot=0x0000000100d09888, b=4308332208, filename="../iseq.c", line=1279) at gc.h:837:5
frame #6: 0x0000000100165b0c miniruby`iseqw_new(iseq=0x0000000100d09880) at iseq.c:1279:9
frame #7: 0x0000000100165a64 miniruby`rb_iseqw_new(iseq=0x0000000100d09880) at iseq.c:1289:12
frame #8: 0x00000001000d8324 miniruby`name_err_init_attr(exc=4309777920, recv=4304780496, method=827660) at error.c:1830:35
frame #9: 0x00000001000d1b80 miniruby`name_err_init(exc=4309777920, mesg=4308332496, recv=4304780496, method=827660) at error.c:1869:12
frame #10: 0x00000001000d1bd4 miniruby`rb_nomethod_err_new(mesg=4308332496, recv=4304780496, method=827660, args=4308332448, priv=0) at error.c:1957:5
frame #11: 0x000000010039049c miniruby`rb_make_no_method_exception(exc=4304914512, format=4308332496, obj=4304780496, argc=1, argv=0x000000016fdfab00, priv=0) at vm_eval.c:959:16
frame #12: 0x00000001003b3274 miniruby`raise_method_missing(ec=0x0000000100b06f40, argc=1, argv=0x000000016fdfab00, obj=4304780496, last_call_status=MISSING_NOENTRY) at vm_eval.c:999:15
frame #13: 0x00000001003945d4 miniruby`rb_method_missing(argc=1, argv=0x000000016fdfab00, obj=4304780496) at vm_eval.c:944:5
...
frame #23: 0x000000010038f5e4 miniruby`rb_vm_call_kw(ec=0x0000000100b06f40, recv=4304780496, id=2865, argc=1, argv=0x000000016fdfab00, me=0x0000000100cbfcf0, kw_splat=0) at vm_eval.c:326:12
frame #24: 0x00000001003c18e4 miniruby`call_method_entry(ec=0x0000000100b06f40, defined_class=4304927952, obj=4304780496, id=2865, cme=0x0000000100cbfcf0, argc=1, argv=0x000000016fdfab00, kw_splat=0) at vm_method.c:2720:20
frame #25: 0x00000001003c440c miniruby`check_funcall_exec(v=6171896792) at vm_eval.c:589:12
frame #26: 0x00000001000dec00 miniruby`rb_vrescue2(b_proc=(miniruby`check_funcall_exec at vm_eval.c:587), data1=6171896792, r_proc=(miniruby`check_funcall_failed at vm_eval.c:596), data2=6171896792, args="Pȗ") at eval.c:919:18
frame #27: 0x00000001000deab0 miniruby`rb_rescue2(b_proc=(miniruby`check_funcall_exec at vm_eval.c:587), data1=6171896792, r_proc=(miniruby`check_funcall_failed at vm_eval.c:596), data2=6171896792) at eval.c:900:17
frame #28: 0x000000010039008c miniruby`check_funcall_missing(ec=0x0000000100b06f40, klass=4304923536, recv=4304780496, mid=3233, argc=0, argv=0x0000000000000000, respond=-1, def=36, kw_splat=0) at vm_eval.c:666:15
frame #29: 0x000000010038fa60 miniruby`rb_check_funcall_default_kw(recv=4304780496, mid=3233, argc=0, argv=0x0000000000000000, def=36, kw_splat=0) at vm_eval.c:703:21
frame #30: 0x000000010038fb04 miniruby`rb_check_funcall(recv=4304780496, mid=3233, argc=0, argv=0x0000000000000000) at vm_eval.c:685:12
frame #31: 0x00000001001c469c miniruby`convert_type_with_id(val=4304780496, tname="String", method=3233, raise=0, index=-1) at object.c:3061:15
frame #32: 0x00000001001c4a4c miniruby`rb_check_convert_type_with_id(val=4304780496, type=5, tname="String", method=3233) at object.c:3153:9
frame #33: 0x00000001002d59f8 miniruby`rb_check_string_type(str=4304780496) at string.c:2571:11
frame #34: 0x000000010014b7b0 miniruby`io_encoding_set(fptr=0x0000000100d09ca0, v1=4304780496, v2=4, opt=4) at io.c:11655:19
frame #35: 0x0000000100139a58 miniruby`rb_io_set_encoding(argc=1, argv=0x000000016fdfb450, io=4308334032) at io.c:13497:5
frame #36: 0x00000001003c0004 miniruby`ractor_safe_call_cfunc_m1(recv=4308334032, argc=1, argv=0x000000016fdfb450, func=(miniruby`rb_io_set_encoding at io.c:13487)) at vm_insnhelper.c:3271:12
...
frame #43: 0x0000000100390b08 miniruby`rb_funcall(recv=4308334032, mid=16593, n=1) at vm_eval.c:1137:12
frame #44: 0x00000001002a43d8 miniruby`load_file_internal(argp_v=6171899936) at ruby.c:2500:5
...
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants