FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Argument forwarding does too much work · Issue #9472 · jruby/jruby · GitHub

/ jruby Public

Argument forwarding does too much work #9472

Description

The current implementation of argument forwarding still works by decomposing the incoming argument list into a set of argument groups and then re-composing those groups back into a varargs-like call. This introduces significant overhead, due to the extra processing overhead and creation of argument lists, but it also seems to be introducing some variability into argument forwarding that can impact behavior.

An example case is from #9462:

def foo(...)
  bar(...)
end
def bar(a)
  p a.object_id
end
x = {a: 1}
p x.object_id
foo(x)

This case failed when JIT compiled because the x hash gets duplicated before the innermost object_id call. But the duplication is happening in the bar method, not as part of the delegated call.

I believe this is because the process of decomposing and recomposing the argument list during delegation is altering the structure of the arguments or the associated callInfo flags in such a way that triggered the JIT bug. That JIT bug will be fixed by #9470, but it is a symptom of the delegation not being a "pure" pass-through.

With the old AST, a delegated method gets parsed as follows:

$ jruby -S ast -e 'def foo(...); bar(...); end' 
AST:
RootNode line: 0
  DefnNode*:foo line: 0
    ArgsNode line: 0
      ListNode line: 0
      ListNode line: 0
      UnnamedRestArgNode:* line: 0
      ListNode line: 0
      ListNode line: 0
      KeywordRestArgNode:** line: 0
      ForwardingBlockArgNode:& line: 0
    FCallNode*:bar line: 0
      ArgsPushNode line: 0
        SplatNode line: 0
          LocalVarNode:* line: 0
        HashNode line: 0, onlykwrest, null
          LocalVarNode:** line: 0
      BlockPassNode line: 0
        ArgsPushNode line: 0
          SplatNode line: 0
            LocalVarNode:* line: 0
          HashNode line: 0, onlykwrest, null
            LocalVarNode:** line: 0
        LocalVarNode:& line: 0

Notice the first thing the method does is process all possible forms of arguments, which then get re-assembled for the delegated call.

The Prism parser provides forwarding as a special AST node:

$ jruby -Xparser.prism -S ast -e 'def foo(...); bar(...); end'
AST:ProgramNode
  locals: 
  statements: StatementsNode
    body: 
      DefNode[Li]
        name: "foo"
        receiver: null
        parameters: ParametersNode
          requireds: 
          optionals: 
          rest: null
          posts: 
          keywords: 
          keyword_rest: ForwardingParameterNode
          block: null
        body: StatementsNode
          body: 
            CallNode
              CallNodeFlags: 33
              receiver: null
              name: "bar"
              arguments: ArgumentsNode
                ArgumentsNodeFlags: 4
                arguments: 
                  ForwardingArgumentsNode
              block: null
        locals: 

But when we compile this to IR, we basically do the same decomposition and recomposition as for the legacy AST:

2026-06-03T00:47:42.496+02:00 [main] INFO InterpretedIRMethod : Printing simple IR for foo:
begin -e::foo
flags: [FLAGS_COMPUTED]
signature(pre=0,opt=0,post=0,rest=STAR,kwargs=0,kwreq=0,kwrest=1)
declared variables:
  *(0:0)
  &(0:2)
  **(0:1)

  00: %self := recv_self
  01:  %v_0 := recv_kw(hasRestArg: true, acceptsKeywords: true)
  02:          check_arity(%v_0, required: 0, opt: 0, rest: true, restKey: 1)
  03:    ** := recv_rest_arg(%v_0, required: 0, argIndex: 0)
  04:   *** := recv_kw_rest_arg(%v_0)
  05:  %v_1 := load_implicit_closure
  06:    *& := reify_closure(%v_1)
  07:          line_num(lineNumber: 0, coverage: false, oneshot: false)
  08:  %v_3 := build_splat(**, dup: true)
  09:  %v_4 := runtime_helper(***, helperMethod: HASH_CHECK)
  10:  %v_5 := build_compound_array(%v_3, %v_4, isArgsPush: true, usesKeywordRest: true)
  11:  %v_6 := build_splat(%v_5, dup: false)
  12:  %v_7 := runtime_helper(splat<%v_6>, helperMethod: IS_HASH_EMPTY)
  13:          b_false(ipc<LBL_0:17>, %v_7, jumpTarget: LBL_0:17, value: %v_7)
  14:  %v_2 := call(self<%self>, *&, callType: FUNCTIONAL, name: bar, potentiallyRefined: false, flags: 7)
  15:          jump(ipc<LBL_1:19>)
  16:          label(ipc<LBL_0:17>)
  17:  %v_2 := call(self<%self>, splat<%v_6>, *&, callType: FUNCTIONAL, name: bar, potentiallyRefined: false, flags: 7)
  18:          label(ipc<LBL_1:19>)
  19:          return(%v_2)

Fixing this in the legacy parser may not be feasible, given its lack of understanding of this new argument construct, but as we move toward Prism as the default we should "pureify" argument forwarding to avoid any modification of the argument list or related call state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


    Back | FazBrowse Home | New Git URL