Skip to content

Commit 063df23

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 0434dfb commit 063df23

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
@@ -4754,6 +4754,22 @@ def entry(define)
47544754
entry(true)
47554755
}
47564756

4757+
assert_equal 'ok', %q{
4758+
def ok
4759+
:ok
4760+
end
4761+
4762+
def delegator(...)
4763+
ok(...)
4764+
end
4765+
4766+
def caller
4767+
send(:delegator)
4768+
end
4769+
4770+
caller
4771+
}
4772+
47574773
assert_equal '[:ok, :ok, :ok]', %q{
47584774
def identity(x) = x
47594775
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

@@ -871,12 +871,12 @@ rb_iseq_compile_node(rb_iseq_t *iseq, const NODE *node)
871871

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

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

1444-
if (!(flag & (VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT)) &&
1444+
if (!(flag & (VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT | VM_CALL_FORWARDING)) &&
14451445
kw_arg == NULL && !has_blockiseq) {
14461446
flag |= VM_CALL_ARGS_SIMPLE;
14471447
}
@@ -2041,6 +2041,13 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
20412041
}
20422042
block_id = args->block_arg;
20432043

2044+
bool optimized_forward = (args->forwarding && args->pre_args_num == 0 && !args->opt_args);
2045+
2046+
if (optimized_forward) {
2047+
rest_id = 0;
2048+
block_id = 0;
2049+
}
2050+
20442051
if (args->opt_args) {
20452052
const rb_node_opt_arg_t *node = args->opt_args;
20462053
LABEL *label;
@@ -2097,7 +2104,7 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
20972104
if (args->kw_args) {
20982105
arg_size = iseq_set_arguments_keywords(iseq, optargs, args, arg_size);
20992106
}
2100-
else if (args->kw_rest_arg) {
2107+
else if (args->kw_rest_arg && !optimized_forward) {
21012108
ID kw_id = iseq->body->local_table[arg_size];
21022109
struct rb_iseq_param_keyword *keyword = ZALLOC_N(struct rb_iseq_param_keyword, 1);
21032110
keyword->rest_start = arg_size++;
@@ -2118,6 +2125,13 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
21182125
iseq_set_use_block(iseq);
21192126
}
21202127

2128+
// Only optimize specifically methods like this: `foo(...)`
2129+
if (optimized_forward) {
2130+
body->param.flags.use_block = 1;
2131+
body->param.flags.forwardable = TRUE;
2132+
arg_size = 1;
2133+
}
2134+
21212135
iseq_calc_param_size(iseq);
21222136
body->param.size = arg_size;
21232137

@@ -2147,13 +2161,26 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
21472161
}
21482162

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

21542181
if (size > 0) {
21552182
ID *ids = (ID *)ALLOC_N(ID, size);
2156-
MEMCPY(ids, tbl->ids, ID, size);
2183+
MEMCPY(ids, tbl->ids + offset, ID, size);
21572184
ISEQ_BODY(iseq)->local_table = ids;
21582185
}
21592186
ISEQ_BODY(iseq)->local_table_size = size;
@@ -4128,7 +4155,7 @@ iseq_specialized_instruction(rb_iseq_t *iseq, INSN *iobj)
41284155
}
41294156
}
41304157

4131-
if ((vm_ci_flag(ci) & VM_CALL_ARGS_BLOCKARG) == 0 && blockiseq == NULL) {
4158+
if ((vm_ci_flag(ci) & (VM_CALL_ARGS_BLOCKARG | VM_CALL_FORWARDING)) == 0 && blockiseq == NULL) {
41324159
iobj->insn_id = BIN(opt_send_without_block);
41334160
iobj->operand_size = insn_len(iobj->insn_id) - 1;
41344161
}
@@ -6291,9 +6318,33 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
62916318
unsigned int dup_rest = 1;
62926319
DECL_ANCHOR(arg_block);
62936320
INIT_ANCHOR(arg_block);
6294-
NO_CHECK(COMPILE(arg_block, "block", RNODE_BLOCK_PASS(argn)->nd_body));
62956321

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

62986349
if (LIST_INSN_SIZE_ONE(arg_block)) {
62996350
LINK_ELEMENT *elem = FIRST_ELEMENT(arg_block);
@@ -6325,7 +6376,7 @@ build_postexe_iseq(rb_iseq_t *iseq, LINK_ANCHOR *ret, const void *ptr)
63256376
ADD_INSN1(ret, body, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
63266377
ADD_CALL_WITH_BLOCK(ret, body, id_core_set_postexe, argc, block);
63276378
RB_OBJ_WRITTEN(iseq, Qundef, (VALUE)block);
6328-
iseq_set_local_table(iseq, 0);
6379+
iseq_set_local_table(iseq, 0, 0);
63296380
}
63306381

63316382
static void
@@ -9420,6 +9471,13 @@ compile_super(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, i
94209471
ADD_GETLOCAL(args, node, idx, lvar_level);
94219472
}
94229473

9474+
/* forward ... */
9475+
if (local_body->param.flags.forwardable) {
9476+
flag |= VM_CALL_FORWARDING;
9477+
int idx = local_body->local_table_size - get_local_var_idx(liseq, idDot3);
9478+
ADD_GETLOCAL(args, node, idx, lvar_level);
9479+
}
9480+
94239481
if (local_body->param.flags.has_opt) {
94249482
/* optional arguments */
94259483
int j;
@@ -12976,7 +13034,8 @@ ibf_dump_iseq_each(struct ibf_dump *dump, const rb_iseq_t *iseq)
1297613034
(body->param.flags.ruby2_keywords << 9) |
1297713035
(body->param.flags.anon_rest << 10) |
1297813036
(body->param.flags.anon_kwrest << 11) |
12979-
(body->param.flags.use_block << 12);
13037+
(body->param.flags.use_block << 12) |
13038+
(body->param.flags.forwardable << 13) ;
1298013039

1298113040
#if IBF_ISEQ_ENABLE_LOCAL_BUFFER
1298213041
# define IBF_BODY_OFFSET(x) (x)
@@ -13193,6 +13252,7 @@ ibf_load_iseq_each(struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t offset)
1319313252
load_body->param.flags.anon_rest = (param_flags >> 10) & 1;
1319413253
load_body->param.flags.anon_kwrest = (param_flags >> 11) & 1;
1319513254
load_body->param.flags.use_block = (param_flags >> 12) & 1;
13255+
load_body->param.flags.forwardable = (param_flags >> 13) & 1;
1319613256
load_body->param.size = param_size;
1319713257
load_body->param.lead_num = param_lead_num;
1319813258
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)