Skip to content

Commit 1f117e2

Browse files
tenderlovejhawthornXrXr
committed
Optimized forwarding callers and callees
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](ruby#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>
1 parent 7c0cf71 commit 1f117e2

32 files changed

Lines changed: 728 additions & 106 deletions

bootstraptest/test_method.rb

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,3 +1176,140 @@ def foo
11761176
foo
11771177
foo
11781178
}, '[Bug #20178]'
1179+
1180+
assert_equal 'ok', %q{
1181+
def bar(x); x; end
1182+
def foo(...); bar(...); end
1183+
foo('ok')
1184+
}
1185+
1186+
assert_equal 'ok', %q{
1187+
def bar(x); x; end
1188+
def foo(z, ...); bar(...); end
1189+
foo(1, 'ok')
1190+
}
1191+
1192+
assert_equal 'ok', %q{
1193+
def bar(x, y); x; end
1194+
def foo(...); bar("ok", ...); end
1195+
foo(1)
1196+
}
1197+
1198+
assert_equal 'ok', %q{
1199+
def bar(x); x; end
1200+
def foo(...); 1.times { return bar(...) }; end
1201+
foo("ok")
1202+
}
1203+
1204+
assert_equal 'ok', %q{
1205+
def bar(x); x; end
1206+
def foo(...); x = nil; 1.times { x = bar(...) }; x; end
1207+
foo("ok")
1208+
}
1209+
1210+
assert_equal 'ok', %q{
1211+
def bar(x); yield; end
1212+
def foo(...); bar(...); end
1213+
foo(1) { "ok" }
1214+
}
1215+
1216+
assert_equal 'ok', %q{
1217+
def baz(x); x; end
1218+
def bar(...); baz(...); end
1219+
def foo(...); bar(...); end
1220+
foo("ok")
1221+
}
1222+
1223+
assert_equal '[1, 2, 3, 4]', %q{
1224+
def baz(a, b, c, d); [a, b, c, d]; end
1225+
def bar(...); baz(1, ...); end
1226+
def foo(...); bar(2, ...); end
1227+
foo(3, 4)
1228+
}
1229+
1230+
assert_equal 'ok', %q{
1231+
class Foo; def self.foo(x); x; end; end
1232+
class Bar < Foo; def self.foo(...); super; end; end
1233+
Bar.foo('ok')
1234+
}
1235+
1236+
assert_equal 'ok', %q{
1237+
class Foo; def self.foo(x); x; end; end
1238+
class Bar < Foo; def self.foo(...); super(...); end; end
1239+
Bar.foo('ok')
1240+
}
1241+
1242+
assert_equal 'ok', %q{
1243+
class Foo; def self.foo(x, y); x + y; end; end
1244+
class Bar < Foo; def self.foo(...); super("o", ...); end; end
1245+
Bar.foo('k')
1246+
}
1247+
1248+
assert_equal 'ok', %q{
1249+
def bar(a); a; end
1250+
def foo(...); lambda { bar(...) }; end
1251+
foo("ok").call
1252+
}
1253+
1254+
assert_equal 'ok', %q{
1255+
class Foo; def self.foo(x, y); x + y; end; end
1256+
class Bar < Foo; def self.y(&b); b; end; def self.foo(...); y { super("o", ...) }; end; end
1257+
Bar.foo('k').call
1258+
}
1259+
1260+
assert_equal 'ok', %q{
1261+
def baz(n); n; end
1262+
def foo(...); bar = baz(...); lambda { lambda { bar } }; end
1263+
foo("ok").call.call
1264+
}
1265+
1266+
assert_equal 'ok', %q{
1267+
class A; def self.foo(...); new(...); end; attr_reader :b; def initialize(a, b:"ng"); @a = a; @b = b; end end
1268+
A.foo(1).b
1269+
A.foo(1, b: "ok").b
1270+
}
1271+
1272+
assert_equal 'ok', %q{
1273+
class A; def initialize; @a = ["ok"]; end; def first(...); @a.first(...); end; end
1274+
def call x; x.first; end
1275+
def call1 x; x.first(1); end
1276+
call(A.new)
1277+
call1(A.new).first
1278+
}
1279+
1280+
assert_equal 'ok', %q{
1281+
class A; def foo; yield("o"); end; end
1282+
class B < A; def foo(...); super { |x| yield(x + "k") }; end; end
1283+
B.new.foo { |x| x }
1284+
}
1285+
1286+
assert_equal "[1, 2, 3, 4]", %q{
1287+
def foo(*b) = b
1288+
1289+
def forward(...)
1290+
splat = [1,2,3]
1291+
foo(*splat, ...)
1292+
end
1293+
1294+
forward(4)
1295+
}
1296+
1297+
assert_equal "[1, 2, 3, 4]", %q{
1298+
class A
1299+
def foo(*b) = b
1300+
end
1301+
1302+
class B < A
1303+
def foo(...)
1304+
splat = [1,2,3]
1305+
super(*splat, ...)
1306+
end
1307+
end
1308+
1309+
B.new.foo(4)
1310+
}
1311+
1312+
assert_equal 'ok', %q{
1313+
class A; attr_reader :iv; def initialize(...) = @iv = "ok"; end
1314+
A.new("foo", bar: []).iv
1315+
}

bootstraptest/test_yjit.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4767,6 +4767,22 @@ def entry(define)
47674767
entry(true)
47684768
}
47694769

4770+
assert_equal 'ok', %q{
4771+
def ok
4772+
:ok
4773+
end
4774+
4775+
def delegator(...)
4776+
ok(...)
4777+
end
4778+
4779+
def caller
4780+
send(:delegator)
4781+
end
4782+
4783+
caller
4784+
}
4785+
47704786
assert_equal '[:ok, :ok, :ok]', %q{
47714787
def identity(x) = x
47724788
def foo(x, _) = x

compile.c

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ static int iseq_setup_insn(rb_iseq_t *iseq, LINK_ANCHOR *const anchor);
490490
static int iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor);
491491
static int iseq_insns_unification(rb_iseq_t *iseq, LINK_ANCHOR *const anchor);
492492

493-
static int iseq_set_local_table(rb_iseq_t *iseq, const rb_ast_id_table_t *tbl);
493+
static int iseq_set_local_table(rb_iseq_t *iseq, const rb_ast_id_table_t *tbl, const NODE *const node_args);
494494
static int iseq_set_exception_local_table(rb_iseq_t *iseq);
495495
static int iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const anchor, const NODE *const node);
496496

@@ -872,12 +872,12 @@ rb_iseq_compile_node(rb_iseq_t *iseq, const NODE *node)
872872

873873
if (node == 0) {
874874
NO_CHECK(COMPILE(ret, "nil", node));
875-
iseq_set_local_table(iseq, 0);
875+
iseq_set_local_table(iseq, 0, 0);
876876
}
877877
/* assume node is T_NODE */
878878
else if (nd_type_p(node, NODE_SCOPE)) {
879879
/* iseq type of top, method, class, block */
880-
iseq_set_local_table(iseq, RNODE_SCOPE(node)->nd_tbl);
880+
iseq_set_local_table(iseq, RNODE_SCOPE(node)->nd_tbl, (NODE *)RNODE_SCOPE(node)->nd_args);
881881
iseq_set_arguments(iseq, ret, (NODE *)RNODE_SCOPE(node)->nd_args);
882882

883883
switch (ISEQ_BODY(iseq)->type) {
@@ -1442,7 +1442,7 @@ new_callinfo(rb_iseq_t *iseq, ID mid, int argc, unsigned int flag, struct rb_cal
14421442
{
14431443
VM_ASSERT(argc >= 0);
14441444

1445-
if (!(flag & (VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT)) &&
1445+
if (!(flag & (VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT | VM_CALL_FORWARDING)) &&
14461446
kw_arg == NULL && !has_blockiseq) {
14471447
flag |= VM_CALL_ARGS_SIMPLE;
14481448
}
@@ -2037,6 +2037,13 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
20372037
}
20382038
block_id = args->block_arg;
20392039

2040+
bool optimized_forward = (args->forwarding && args->pre_args_num == 0 && !args->opt_args);
2041+
2042+
if (optimized_forward) {
2043+
rest_id = 0;
2044+
block_id = 0;
2045+
}
2046+
20402047
if (args->opt_args) {
20412048
const rb_node_opt_arg_t *node = args->opt_args;
20422049
LABEL *label;
@@ -2093,7 +2100,7 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
20932100
if (args->kw_args) {
20942101
arg_size = iseq_set_arguments_keywords(iseq, optargs, args, arg_size);
20952102
}
2096-
else if (args->kw_rest_arg) {
2103+
else if (args->kw_rest_arg && !optimized_forward) {
20972104
ID kw_id = iseq->body->local_table[arg_size];
20982105
struct rb_iseq_param_keyword *keyword = ZALLOC_N(struct rb_iseq_param_keyword, 1);
20992106
keyword->rest_start = arg_size++;
@@ -2114,6 +2121,13 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
21142121
iseq_set_use_block(iseq);
21152122
}
21162123

2124+
// Only optimize specifically methods like this: `foo(...)`
2125+
if (optimized_forward) {
2126+
body->param.flags.use_block = 1;
2127+
body->param.flags.forwardable = TRUE;
2128+
arg_size = 1;
2129+
}
2130+
21172131
iseq_calc_param_size(iseq);
21182132
body->param.size = arg_size;
21192133

@@ -2143,13 +2157,26 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
21432157
}
21442158

21452159
static int
2146-
iseq_set_local_table(rb_iseq_t *iseq, const rb_ast_id_table_t *tbl)
2160+
iseq_set_local_table(rb_iseq_t *iseq, const rb_ast_id_table_t *tbl, const NODE *const node_args)
21472161
{
21482162
unsigned int size = tbl ? tbl->size : 0;
2163+
unsigned int offset = 0;
2164+
2165+
if (node_args) {
2166+
struct rb_args_info *args = &RNODE_ARGS(node_args)->nd_ainfo;
2167+
2168+
// If we have a function that only has `...` as the parameter,
2169+
// then its local table should only be `...`
2170+
// FIXME: I think this should be fixed in the AST rather than special case here.
2171+
if (args->forwarding && args->pre_args_num == 0 && !args->opt_args) {
2172+
size -= 3;
2173+
offset += 3;
2174+
}
2175+
}
21492176

21502177
if (size > 0) {
21512178
ID *ids = (ID *)ALLOC_N(ID, size);
2152-
MEMCPY(ids, tbl->ids, ID, size);
2179+
MEMCPY(ids, tbl->ids + offset, ID, size);
21532180
ISEQ_BODY(iseq)->local_table = ids;
21542181
}
21552182
ISEQ_BODY(iseq)->local_table_size = size;
@@ -4124,7 +4151,7 @@ iseq_specialized_instruction(rb_iseq_t *iseq, INSN *iobj)
41244151
}
41254152
}
41264153

4127-
if ((vm_ci_flag(ci) & VM_CALL_ARGS_BLOCKARG) == 0 && blockiseq == NULL) {
4154+
if ((vm_ci_flag(ci) & (VM_CALL_ARGS_BLOCKARG | VM_CALL_FORWARDING)) == 0 && blockiseq == NULL) {
41284155
iobj->insn_id = BIN(opt_send_without_block);
41294156
iobj->operand_size = insn_len(iobj->insn_id) - 1;
41304157
}
@@ -6287,9 +6314,33 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
62876314
unsigned int dup_rest = 1;
62886315
DECL_ANCHOR(arg_block);
62896316
INIT_ANCHOR(arg_block);
6290-
NO_CHECK(COMPILE(arg_block, "block", RNODE_BLOCK_PASS(argn)->nd_body));
62916317

6292-
*flag |= VM_CALL_ARGS_BLOCKARG;
6318+
if (RNODE_BLOCK_PASS(argn)->forwarding && ISEQ_BODY(ISEQ_BODY(iseq)->local_iseq)->param.flags.forwardable) {
6319+
int idx = ISEQ_BODY(ISEQ_BODY(iseq)->local_iseq)->local_table_size;// - get_local_var_idx(iseq, idDot3);
6320+
6321+
RUBY_ASSERT(nd_type_p(RNODE_BLOCK_PASS(argn)->nd_head, NODE_ARGSPUSH));
6322+
const NODE * arg_node =
6323+
RNODE_ARGSPUSH(RNODE_BLOCK_PASS(argn)->nd_head)->nd_head;
6324+
6325+
int argc = 0;
6326+
6327+
// Only compile leading args:
6328+
// foo(x, y, ...)
6329+
// ^^^^
6330+
if (nd_type_p(arg_node, NODE_ARGSCAT)) {
6331+
argc += setup_args_core(iseq, args, RNODE_ARGSCAT(arg_node)->nd_head, dup_rest, flag, keywords);
6332+
}
6333+
6334+
*flag |= VM_CALL_FORWARDING;
6335+
6336+
ADD_GETLOCAL(args, argn, idx, get_lvar_level(iseq));
6337+
return INT2FIX(argc);
6338+
}
6339+
else {
6340+
*flag |= VM_CALL_ARGS_BLOCKARG;
6341+
6342+
NO_CHECK(COMPILE(arg_block, "block", RNODE_BLOCK_PASS(argn)->nd_body));
6343+
}
62936344

62946345
if (LIST_INSN_SIZE_ONE(arg_block)) {
62956346
LINK_ELEMENT *elem = FIRST_ELEMENT(arg_block);
@@ -6321,7 +6372,7 @@ build_postexe_iseq(rb_iseq_t *iseq, LINK_ANCHOR *ret, const void *ptr)
63216372
ADD_INSN1(ret, body, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
63226373
ADD_CALL_WITH_BLOCK(ret, body, id_core_set_postexe, argc, block);
63236374
RB_OBJ_WRITTEN(iseq, Qundef, (VALUE)block);
6324-
iseq_set_local_table(iseq, 0);
6375+
iseq_set_local_table(iseq, 0, 0);
63256376
}
63266377

63276378
static void
@@ -9433,6 +9484,13 @@ compile_super(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, i
94339484
ADD_GETLOCAL(args, node, idx, lvar_level);
94349485
}
94359486

9487+
/* forward ... */
9488+
if (local_body->param.flags.forwardable) {
9489+
flag |= VM_CALL_FORWARDING;
9490+
int idx = local_body->local_table_size - get_local_var_idx(liseq, idDot3);
9491+
ADD_GETLOCAL(args, node, idx, lvar_level);
9492+
}
9493+
94369494
if (local_body->param.flags.has_opt) {
94379495
/* optional arguments */
94389496
int j;
@@ -12989,7 +13047,8 @@ ibf_dump_iseq_each(struct ibf_dump *dump, const rb_iseq_t *iseq)
1298913047
(body->param.flags.ruby2_keywords << 9) |
1299013048
(body->param.flags.anon_rest << 10) |
1299113049
(body->param.flags.anon_kwrest << 11) |
12992-
(body->param.flags.use_block << 12);
13050+
(body->param.flags.use_block << 12) |
13051+
(body->param.flags.forwardable << 13) ;
1299313052

1299413053
#if IBF_ISEQ_ENABLE_LOCAL_BUFFER
1299513054
# define IBF_BODY_OFFSET(x) (x)
@@ -13206,6 +13265,7 @@ ibf_load_iseq_each(struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t offset)
1320613265
load_body->param.flags.anon_rest = (param_flags >> 10) & 1;
1320713266
load_body->param.flags.anon_kwrest = (param_flags >> 11) & 1;
1320813267
load_body->param.flags.use_block = (param_flags >> 12) & 1;
13268+
load_body->param.flags.forwardable = (param_flags >> 13) & 1;
1320913269
load_body->param.size = param_size;
1321013270
load_body->param.lead_num = param_lead_num;
1321113271
load_body->param.opt_num = param_opt_num;

imemo.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating)
316316
else {
317317
if (vm_cc_super_p(cc) || vm_cc_refinement_p(cc)) {
318318
rb_gc_mark_movable((VALUE)cc->cme_);
319+
rb_gc_mark_movable((VALUE)cc->klass);
319320
}
320321
}
321322

0 commit comments

Comments
 (0)