| Age | Commit message (Collapse) | Author |
|---|
| This supports the nodes in both in the parse.y and prism compilers. Fixes [Bug #20043] Co-authored-by: Kevin Newton <kddnewton@gmail.com> |
| This commit adds `sendforward` and `invokesuperforward` for forwarding parameters to calls Co-authored-by: Matt Valentine-House <matt@eightbitraptor.com> |
| This patch optimizes forwarding callers and callees. It only optimizes methods that only take `...` as their parameter, and then pass `...` to other calls. Calls it optimizes look like this: ```ruby def bar(a) = a def foo(...) = bar(...) # optimized foo(123) ``` ```ruby def bar(a) = a def foo(...) = bar(1, 2, ...) # optimized foo(123) ``` ```ruby def bar(*a) = a def foo(...) list = [1, 2] bar(*list, ...) # optimized end foo(123) ``` All variants of the above but using `super` are also optimized, including a bare super like this: ```ruby def foo(...) super end ``` This patch eliminates intermediate allocations made when calling methods that accept `...`. We can observe allocation elimination like this: ```ruby def m x = GC.stat(:total_allocated_objects) yield GC.stat(:total_allocated_objects) - x end def bar(a) = a def foo(...) = bar(...) def test m { foo(123) } end test p test # allocates 1 object on master, but 0 objects with this patch ``` ```ruby def bar(a, b:) = a + b def foo(...) = bar(...) def test m { foo(1, b: 2) } end test p test # allocates 2 objects on master, but 0 objects with this patch ``` How does it work? ----------------- This patch works by using a dynamic stack size when passing forwarded parameters to callees. The caller's info object (known as the "CI") contains the stack size of the parameters, so we pass the CI object itself as a parameter to the callee. When forwarding parameters, the forwarding ISeq uses the caller's CI to determine how much stack to copy, then copies the caller's stack before calling the callee. The CI at the forwarded call site is adjusted using information from the caller's CI. I think this description is kind of confusing, so let's walk through an example with code. ```ruby def delegatee(a, b) = a + b def delegator(...) delegatee(...) # CI2 (FORWARDING) end def caller delegator(1, 2) # CI1 (argc: 2) end ``` Before we call the delegator method, the stack looks like this: ``` Executing Line | Code | Stack ---------------+---------------------------------------+-------- 1| def delegatee(a, b) = a + b | self 2| | 1 3| def delegator(...) | 2 4| # | 5| delegatee(...) # CI2 (FORWARDING) | 6| end | 7| | 8| def caller | -> 9| delegator(1, 2) # CI1 (argc: 2) | 10| end | ``` The ISeq for `delegator` is tagged as "forwardable", so when `caller` calls in to `delegator`, it writes `CI1` on to the stack as a local variable for the `delegator` method. The `delegator` method has a special local called `...` that holds the caller's CI object. Here is the ISeq disasm fo `delegator`: ``` == disasm: #<ISeq:delegator@-e:1 (1,0)-(1,39)> local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) [ 1] "..."@0 0000 putself ( 1)[LiCa] 0001 getlocal_WC_0 "..."@0 0003 send <calldata!mid:delegatee, argc:0, FCALL|FORWARDING>, nil 0006 leave [Re] ``` The local called `...` will contain the caller's CI: CI1. Here is the stack when we enter `delegator`: ``` Executing Line | Code | Stack ---------------+---------------------------------------+-------- 1| def delegatee(a, b) = a + b | self 2| | 1 3| def delegator(...) | 2 -> 4| # | CI1 (argc: 2) 5| delegatee(...) # CI2 (FORWARDING) | cref_or_me 6| end | specval 7| | type 8| def caller | 9| delegator(1, 2) # CI1 (argc: 2) | 10| end | ``` The CI at `delegatee` on line 5 is tagged as "FORWARDING", so it knows to memcopy the caller's stack before calling `delegatee`. In this case, it will memcopy self, 1, and 2 to the stack before calling `delegatee`. It knows how much memory to copy from the caller because `CI1` contains stack size information (argc: 2). Before executing the `send` instruction, we push `...` on the stack. The `send` instruction pops `...`, and because it is tagged with `FORWARDING`, it knows to memcopy (using the information in the CI it just popped): ``` == disasm: #<ISeq:delegator@-e:1 (1,0)-(1,39)> local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) [ 1] "..."@0 0000 putself ( 1)[LiCa] 0001 getlocal_WC_0 "..."@0 0003 send <calldata!mid:delegatee, argc:0, FCALL|FORWARDING>, nil 0006 leave [Re] ``` Instruction 001 puts the caller's CI on the stack. `send` is tagged with FORWARDING, so it reads the CI and _copies_ the callers stack to this stack: ``` Executing Line | Code | Stack ---------------+---------------------------------------+-------- 1| def delegatee(a, b) = a + b | self 2| | 1 3| def delegator(...) | 2 4| # | CI1 (argc: 2) -> 5| delegatee(...) # CI2 (FORWARDING) | cref_or_me 6| end | specval 7| | type 8| def caller | self 9| delegator(1, 2) # CI1 (argc: 2) | 1 10| end | 2 ``` The "FORWARDING" call site combines information from CI1 with CI2 in order to support passing other values in addition to the `...` value, as well as perfectly forward splat args, kwargs, etc. Since we're able to copy the stack from `caller` in to `delegator`'s stack, we can avoid allocating objects. I want to do this to eliminate object allocations for delegate methods. My long term goal is to implement `Class#new` in Ruby and it uses `...`. I was able to implement `Class#new` in Ruby [here](https://github.com/ruby/ruby/pull/9289). If we adopt the technique in this patch, then we can optimize allocating objects that take keyword parameters for `initialize`. For example, this code will allocate 2 objects: one for `SomeObject`, and one for the kwargs: ```ruby SomeObject.new(foo: 1) ``` If we combine this technique, plus implement `Class#new` in Ruby, then we can reduce allocations for this common operation. Co-Authored-By: John Hawthorn <john@hawthorn.email> Co-Authored-By: Alan Wu <XrXr@users.noreply.github.com> |
| |
| [Bug #20569] `putobject RubyVM::FrozenCore`, is not serializable, we have to use `putspecialobject VM_SPECIAL_OBJECT_VMCORE`. |
| Ref: https://github.com/ruby/ruby/pull/10872 These should be the last internal uses of the old `Data` API inside Ruby itself. Some use remain in a couple default gems. |
| |
| Previously, ensure ISEQs took their first line number from the line number coming from the AST. However, if this is coming from an empty `begin`..`end` inside of a method, this can be all of the way back to the method declaration. Instead, this commit changes it to be the first line number of the ensure block itself. The first_lineno field is only accessible through manual ISEQ compilation or through tracepoint. Either way, this will be more accurate for targeting going forward. |
| With embedded strings we often have some space left in the slot, which we can use to store the string Hash code. It's probably only worth it for string literals, as they are the ones likely to be used as hash keys. We chose to store the Hash code right after the string terminator as to make it easy/fast to compute, and not require one more union in RString. ``` compare-ruby: ruby 3.4.0dev (2024-04-22T06:32:21Z main f77618c1fa) [arm64-darwin23] built-ruby: ruby 3.4.0dev (2024-04-22T10:13:03Z interned-string-ha.. 8a1a32331b) [arm64-darwin23] last_commit=Precompute embedded string literals hash code | |compare-ruby|built-ruby| |:-----------|-----------:|---------:| |symbol | 39.275M| 39.753M| | | -| 1.01x| |dyn_symbol | 37.348M| 37.704M| | | -| 1.01x| |small_lit | 29.514M| 33.948M| | | -| 1.15x| |frozen_lit | 27.180M| 33.056M| | | -| 1.22x| |iseq_lit | 27.391M| 32.242M| | | -| 1.18x| ``` Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com> |
| |
| Instructions for this code: ```ruby # frozen_string_literal: true [a].pack("C") ``` Before this commit: ``` == disasm: #<ISeq:<main>@test.rb:1 (1,0)-(3,13)> 0000 putself ( 3)[Li] 0001 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0003 newarray 1 0005 putobject "C" 0007 opt_send_without_block <calldata!mid:pack, argc:1, ARGS_SIMPLE> 0009 leave ``` After this commit: ``` == disasm: #<ISeq:<main>@test.rb:1 (1,0)-(3,13)> 0000 putself ( 3)[Li] 0001 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0003 putobject "C" 0005 opt_newarray_send 2, :pack 0008 leave ``` Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com> Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org> |
| |
| There is an English word "vast". This commit changes the name to be more clear name to avoid confusion. |
| This patch adds `int line_count` field to `rb_ast_body_t` structure. Instead, we no longer cast `script_lines` to Fixnum. ## Background Ref https://github.com/ruby/ruby/pull/10618 In the PR above, we have decoupled IMEMO from `rb_ast_t`. This means we could lift the five-words-restriction of the structure that forced us to unionize `rb_ast_t *` and `FIXNUM` in one field. ## Relating refactor - Remove the second parameter of `rb_ruby_ast_new()` function ## Attention I will remove a code that assigns -1 to line_count, in `rb_binding_add_dynavars()` of vm.c, because I don't think it is necessary. But I will make another PR for this so that we can atomically revert in case I was wrong (See the comment on the code) |
| |
| This patch removes the `VALUE flags` member from the `rb_ast_t` structure making `rb_ast_t` no longer an IMEMO object. ## Background We are trying to make the Ruby parser generated from parse.y a universal parser that can be used by other implementations such as mruby. To achieve this, it is necessary to exclude VALUE and IMEMO from parse.y, AST, and NODE. ## Summary (file by file) - `rubyparser.h` - Remove the `VALUE flags` member from `rb_ast_t` - `ruby_parser.c` and `internal/ruby_parser.h` - Use TypedData_Make_Struct VALUE which wraps `rb_ast_t` `in ast_alloc()` so that GC can manage it - You can retrieve `rb_ast_t` from the VALUE by `rb_ruby_ast_data_get()` - Change the return type of `rb_parser_compile_XXXX()` functions from `rb_ast_t *` to `VALUE` - rb_ruby_ast_new() which internally `calls ast_alloc()` is to create VALUE vast outside ruby_parser.c - `iseq.c` and `vm_core.h` - Amend the first parameter of `rb_iseq_new_XXXX()` functions from `rb_ast_body_t *` to `VALUE` - This keeps the VALUE of AST on the machine stack to prevent being removed by GC - `ast.c` - Almost all change is replacement `rb_ast_t *ast` with `VALUE vast` (sorry for the big diff) - Fix `node_memsize()` - Now it includes `rb_ast_local_table_link`, `tokens` and script_lines - `compile.c`, `load.c`, `node.c`, `parse.y`, `proc.c`, `ruby.c`, `template/prelude.c.tmpl`, `vm.c` and `vm_eval.c` - Follow-up due to the above changes - `imemo.{c|h}` - If an object with `imemo_ast` appears, considers it a bug Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org> |
| In cases where RubyVM::InstructionSequence.load_from_binary() is passed a param other than a String, we attempt to call the RSTRING_LENINT macro on it which can cause a segfault. ex: ``` var_0 = 0 RubyVM::InstructionSequence.load_from_binary(var_0) ``` This commit adds a type check to raise unless we are provided a String. |
| `RUBY_TRY_UNUSED_BLOCK_WARNING_STRICT=1 ruby ...` will enable strict check for unused block warning. This option is only for trial to compare the results so the envname is not considered well. Should be removed before Ruby 3.4.0 release. |
| if a method `foo` uses a block, other (unrelated) method `foo` can receives a block. So try to relax the unused block warning condition. ```ruby class C0 def f = yield end class C1 < C0 def f = nil end [C0, C1].f{ block } # do not warn ``` |
| ```ruby b = RubyVM::InstructionSequence.compile('def f = yield; def g = nil').to_a pp b #=> ... {:use_block=>true}, ... ``` |
| Previously it would bypass the `FL_ABLE` check, but since shapes introduction, it started having a different behavior than `OBJ_FREEZE`, as it would onyl set the `FL_FREEZE` flag, but not update the shape. I have no indication of this causing a bug yet, but it seems like a trap waiting to happen. |
| This patch is part of universal parser work. ## Summary - Decouple VALUE from members below: - `(struct parser_params *)->debug_lines` - `(rb_ast_t *)->body.script_lines` - Instead, they are now `rb_parser_ary_t *` - They can also be a `(VALUE)FIXNUM` as before to hold line count - `ISEQ_BODY(iseq)->variable.script_lines` remains VALUE - In order to do this, - Add `VALUE script_lines` param to `rb_iseq_new_with_opt()` - Introduce `rb_parser_build_script_lines_from()` to convert `rb_parser_ary_t *` into `VALUE` ## Other details - Extend `rb_parser_ary_t *`. It previously could only store `rb_parser_ast_token *`, now can store script_lines, too - Change tactics of building the top-level `SCRIPT_LINES__` in `yycompile0()` - Before: While parsing, each line of the script is added to `SCRIPT_LINES__[path]` - After: After `yyparse(p)`, `SCRIPT_LINES__[path]` will be built from `p->debug_lines` - Remove the second parameter of `rb_parser_set_script_lines()` to make it simple - Introduce `script_lines_free()` to be called from `rb_ast_free()` because the GC no longer takes care of the script_lines - Introduce `rb_parser_string_deep_copy()` in parse.y to maintain script_lines when `rb_ruby_parser_free()` called - With regard to this, please see *Future tasks* below ## Future tasks - Decouple IMEMO from `rb_ast_t *` - This lifts the five-members-restriction of Ruby object, - So we will be able to move the ownership of the `lex.string_buffer` from parser to AST - Then we remove `rb_parser_string_deep_copy()` to make the whole thing simple |
| `super(){}`, `super{}` and `super(&b)` doesn't use the given block so warn unused block warning when calling a method which doesn't use block with above `super` expressions. e.g.: `def f = super{B1}` (warn on `f{B2}` because `B2` is not used. |
| `super()` (not zsuper) passes the passed block and it can be used. ```ruby class C0 def foo; yield; end end class C1 < C0 def foo; super(); end end C1.new.foo{p :block} #=> :block ``` |
| With verbopse mode (-w), the interpreter shows a warning if a block is passed to a method which does not use the given block. Warning on: * the invoked method is written in C * the invoked method is not `initialize` * not invoked with `super` * the first time on the call-site with the invoked method (`obj.foo{}` will be warned once if `foo` is same method) [Feature #15554] `Primitive.attr! :use_block` is introduced to declare that primitive functions (written in C) will use passed block. For minitest, test needs some tweak, so use https://github.com/minitest/minitest/commit/ea9caafc0754b1d6236a490d59e624b53209734a for `test-bundled-gems`. |
| The `rb_fstring(rb_enc_str_new())` pattern is inneficient because: - It passes a mutable string to `rb_fstring` so if it has to be interned it will first be duped. - It an equivalent interned string already exists, we allocated the string for nothing. With `rb_enc_interned_str` we either directly get the pre-existing string with 0 allocations, or efficiently directly intern the one we create without first duping it. |
| Passing blocks is no longer allowed in attrasgn. This is similar to 3a674c9c655288b3e12ac1cff149ba4af08fd452, but for attrasgn instead of op_asgn. |
| In the past, `rb_iseq_compile_node` received `NODE *` and `struct vm_ifunc *` as `node`. But after e743a35, the function only receives `NODE *`. This commit removes imemo type check to reduce the dependence on `VALUE flags` of `struct RNode`. |
| Code such as: ```ruby foo[0, &bar] = baz foo[0, bar: 1] = baz foo[0, **bar] = baz ``` Is now a syntax error, so all of the removed code is now dead. |
| This patch surppresses the warning below: ```console compile.c:10314:1: warning: control reaches end of non-void function [-Wreturn-type] 10314 | } | ^ ``` |
| |
| |
| I discovered the problem in `compile.c` from a failing TestIseqLoad#test_stressful_roundtrip test with ASAN enabled. The other two changes in array.c and string.c I found by auditing similar usages of DATA_PTR in the codebase. [Bug #20402] |
| * YJIT: add iseq_alloc_count to stats * Remove an empty line --------- Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com> |
| [Feature #20205] As a path toward enabling frozen string literals by default in the future, this commit introduce "chilled strings". From a user perspective chilled strings pretend to be frozen, but on the first attempt to mutate them, they lose their frozen status and emit a warning rather than to raise a `FrozenError`. Implementation wise, `rb_compile_option_struct.frozen_string_literal` is no longer a boolean but a tri-state of `enabled/disabled/unset`. When code is compiled with frozen string literals neither explictly enabled or disabled, string literals are compiled with a new `putchilledstring` instruction. This instruction is identical to `putstring` except it marks the String with the `STR_CHILLED (FL_USER3)` and `FL_FREEZE` flags. Chilled strings have the `FL_FREEZE` flag as to minimize the need to check for chilled strings across the codebase, and to improve compatibility with C extensions. Notes: - `String#freeze`: clears the chilled flag. - `String#-@`: acts as if the string was mutable. - `String#+@`: acts as if the string was mutable. - `String#clone`: copies the chilled flag. Co-authored-by: Jean Boussier <byroot@ruby-lang.org> |
| Previously, this used: ``` splatarray false duphash getlocal/getblockparamproxy # in the block passing case send ARGS_SPLAT|KW_SPLAT|KW_SPLAT_MUT ``` This changes the duphash to putobject, with putobject using a frozen version of the hash, and removing the keyword mutability: ``` splatarray false putobject getlocal/getblockparamproxy # in the block passing case send ARGS_SPLAT|KW_SPLAT ``` |
| In preparation for https://bugs.ruby-lang.org/issues/20205. The `frozen_string_literal` compilation option will no longer be a boolean but a tri-state: `on/off/default`. |
| This was broken during the refactoring in 22e488464a412afa58f201c49e54773aa8011320. |
| This fixes test failures when running tests with RUBY_ISEQ_DUMP_DEBUG=to_binary, which started after 5899f6aa55a02f211545d9cdaef4d86fa0b49528 was committed. Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org> |
| Fixes [Bug #19975] |
| |
| In cases where a method accepts both keywords and an anonymous keyword splat, the method was not marked as taking an anonymous keyword splat. Fix that in the compiler. Doing that broke handling of nil keyword splats in yjit, so update yjit to handle that. Add a test to check that calling a method that accepts both a keyword argument and an anonymous keyword splat does not modify a passed keyword splat hash. Move the anon_kwrest check from setup_parameters_complex to ignore_keyword_hash_p, and only use it if the keyword hash is already a hash. This should speed things up slightly as it avoids a check previously used for all callers of setup_parameters_complex. Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org> |
| Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org> |
| These previously resulted in 2 array allocations, one for newarray and one for concatarray. This replaces newarray and concatarray with pushtoarray, and changes splatarray false to splatarray true, which reduces it to 1 array allocation, in splatarray true. This also sets VM_CALL_ARGS_SPLAT_MUT, so if the super method accepts a positional splat, this will avoid an additional array allocation on the callee side. |
| This optimization stopped being using when the splatkw VM instruction was added. This change allows the optimization to apply again. This also optimizes the following cases: super(*ary, **kw, &block) f(...) super(...) |
| to send This optimizes cases such as: super(arg, *ary) super(arg, *ary, &block) super(*ary, **kw) super(*ary, kw: 1) super(*ary, kw: 1, &block) The super(*ary, **kw, &block) case does not use the splatarray false optimization. This is also true of the send case, since the introduction of the splatkw VM instruction. That will be fixed in a later commit. |
| |
| We should pass `false` for `hash_key` for value nodes. Credits to `@kddnewton` for noticing and bisecting. |
| |
| This is designed to replace the newarraykwsplat instruction, which is no longer used in the parse.y compiler after this commit. This avoids an unnecessary array allocation in the case where ARGSCAT is followed by LIST with keyword: ```ruby a = [] kw = {} [*a, 1, **kw] ``` Previous Instructions: ``` 0000 newarray 0 ( 1)[Li] 0002 setlocal_WC_0 a@0 0004 newhash 0 ( 2)[Li] 0006 setlocal_WC_0 kw@1 0008 getlocal_WC_0 a@0 ( 3)[Li] 0010 splatarray true 0012 putobject_INT2FIX_1_ 0013 putspecialobject 1 0015 newhash 0 0017 getlocal_WC_0 kw@1 0019 opt_send_without_block <calldata!mid:core#hash_merge_kwd, argc:2, ARGS_SIMPLE> 0021 newarraykwsplat 2 0023 concattoarray 0024 leave ``` New Instructions: ``` 0000 newarray 0 ( 1)[Li] 0002 setlocal_WC_0 a@0 0004 newhash 0 ( 2)[Li] 0006 setlocal_WC_0 kw@1 0008 getlocal_WC_0 a@0 ( 3)[Li] 0010 splatarray true 0012 putobject_INT2FIX_1_ 0013 pushtoarray 1 0015 putspecialobject 1 0017 newhash 0 0019 getlocal_WC_0 kw@1 0021 opt_send_without_block <calldata!mid:core#hash_merge_kwd, argc:2, ARGS_SIMPLE> 0023 pushtoarraykwsplat 0024 leave ``` pushtoarraykwsplat is designed to be simpler than newarraykwsplat. It does not take a variable number of arguments from the stack, it pops the top of the stack, and appends it to the second from the top, unless the top of the stack is an empty hash. During this work, I found the ARGSPUSH followed by HASH with keyword did not compile correctly, as it pushed the generated hash to the array even if the hash was empty. This fixes the behavior, to use pushtoarraykwsplat instead of pushtoarray in that case: ```ruby a = [] kw = {} [*a, **kw] [{}] # Before [] # After ``` This does not remove the newarraykwsplat instruction, as it is still referenced in the prism compiler (which should be updated similar to this), YJIT (only in the bindings, it does not appear to be implemented), and RJIT (in a couple comments). After those are updated, the newarraykwsplat instruction should be removed. |
