ruby-changes:65697
From: Jeremy <ko1@a...>
Date: Mon, 29 Mar 2021 23:45:38 +0900 (JST)
Subject: [ruby-changes:65697] 7b3c5ab8a5 (master): Make defined? cache the results of method calls
https://git.ruby-lang.org/ruby.git/commit/?id=7b3c5ab8a5 From 7b3c5ab8a5825a2b960e639d257f0c8a69c4186c Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Mon, 22 Feb 2021 14:28:40 -0800 Subject: Make defined? cache the results of method calls Previously, defined? could result in many more method calls than the code it was checking. `defined? a.b.c.d.e.f` generated 15 calls, with `a` called 5 times, `b` called 4 times, etc.. This was due to the fact that defined works in a recursive manner, but it previously did not cache results. So for `defined? a.b.c.d.e.f`, the logic was similar to ```ruby return nil unless defined? a return nil unless defined? a.b return nil unless defined? a.b.c return nil unless defined? a.b.c.d return nil unless defined? a.b.c.d.e return nil unless defined? a.b.c.d.e.f "method" ``` With this change, the logic is similar to the following, without the creation of a local variable: ```ruby return nil unless defined? a _ = a return nil unless defined? _.b _ = _.b return nil unless defined? _.c _ = _.c return nil unless defined? _.d _ = _.d return nil unless defined? _.e _ = _.e return nil unless defined? _.f "method" ``` In addition to eliminating redundant method calls for defined statements, this greatly simplifies the instruction sequences by eliminating duplication. Previously: ``` 0000 putnil ( 1)[Li] 0001 putself 0002 defined func, :a, false 0006 branchunless 73 0008 putself 0009 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0011 defined method, :b, false 0015 branchunless 73 0017 putself 0018 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0020 opt_send_without_block <calldata!mid:b, argc:0, ARGS_SIMPLE> 0022 defined method, :c, false 0026 branchunless 73 0028 putself 0029 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0031 opt_send_without_block <calldata!mid:b, argc:0, ARGS_SIMPLE> 0033 opt_send_without_block <calldata!mid:c, argc:0, ARGS_SIMPLE> 0035 defined method, :d, false 0039 branchunless 73 0041 putself 0042 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0044 opt_send_without_block <calldata!mid:b, argc:0, ARGS_SIMPLE> 0046 opt_send_without_block <calldata!mid:c, argc:0, ARGS_SIMPLE> 0048 opt_send_without_block <calldata!mid:d, argc:0, ARGS_SIMPLE> 0050 defined method, :e, false 0054 branchunless 73 0056 putself 0057 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0059 opt_send_without_block <calldata!mid:b, argc:0, ARGS_SIMPLE> 0061 opt_send_without_block <calldata!mid:c, argc:0, ARGS_SIMPLE> 0063 opt_send_without_block <calldata!mid:d, argc:0, ARGS_SIMPLE> 0065 opt_send_without_block <calldata!mid:e, argc:0, ARGS_SIMPLE> 0067 defined method, :f, true 0071 swap 0072 pop 0073 leave ``` After change: ``` 0000 putnil ( 1)[Li] 0001 putself 0002 dup 0003 defined func, :a, false 0007 branchunless 52 0009 opt_send_without_block <calldata!mid:a, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0011 dup 0012 defined method, :b, false 0016 branchunless 52 0018 opt_send_without_block <calldata!mid:b, argc:0, ARGS_SIMPLE> 0020 dup 0021 defined method, :c, false 0025 branchunless 52 0027 opt_send_without_block <calldata!mid:c, argc:0, ARGS_SIMPLE> 0029 dup 0030 defined method, :d, false 0034 branchunless 52 0036 opt_send_without_block <calldata!mid:d, argc:0, ARGS_SIMPLE> 0038 dup 0039 defined method, :e, false 0043 branchunless 52 0045 opt_send_without_block <calldata!mid:e, argc:0, ARGS_SIMPLE> 0047 defined method, :f, true 0051 swap 0052 pop 0053 leave ``` This fixes issues where for pathological small examples, Ruby would generate huge instruction sequences. Unfortunately, implementing this support is kind of a hack. This adds another parameter to compile_call for whether we should assume the receiver is already present on the stack, and has defined? set that parameter for the specific case where it is compiling a method call where the receiver is also a method call. defined_expr0 also takes an additional parameter for whether it should leave the results of the method call on the stack. If that argument is true, in the case where the method isn't defined, we jump to the pop before the leave, so the extra result is not left on the stack. This requires space for an additional label, so lfinish now needs to be able to hold 3 labels. Fixes [Bug #17649] Fixes [Bug #13708] --- compile.c | 99 ++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 66 insertions(+), 33 deletions(-) diff --git a/compile.c b/compile.c index 230b800..7e1724c 100644 --- a/compile.c +++ b/compile.c @@ -4883,9 +4883,13 @@ static void https://github.com/ruby/ruby/blob/trunk/compile.c#L4883 defined_expr(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, LABEL **lfinish, VALUE needstr); +static int +compile_call(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, const enum node_type type, int line, int popped, bool assume_receiver); + static void defined_expr0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, - const NODE *const node, LABEL **lfinish, VALUE needstr) + const NODE *const node, LABEL **lfinish, VALUE needstr, + bool keep_result) { enum defined_type expr_type = DEFINED_NOT_DEFINED; enum node_type type; @@ -4911,7 +4915,7 @@ defined_expr0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, https://github.com/ruby/ruby/blob/trunk/compile.c#L4915 const NODE *vals = node; do { - defined_expr0(iseq, ret, vals->nd_head, lfinish, Qfalse); + defined_expr0(iseq, ret, vals->nd_head, lfinish, Qfalse, false); if (!lfinish[1]) { lfinish[1] = NEW_LABEL(line); @@ -4963,7 +4967,7 @@ defined_expr0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, https://github.com/ruby/ruby/blob/trunk/compile.c#L4967 if (!lfinish[1]) { lfinish[1] = NEW_LABEL(line); } - defined_expr0(iseq, ret, node->nd_head, lfinish, Qfalse); + defined_expr0(iseq, ret, node->nd_head, lfinish, Qfalse, false); ADD_INSNL(ret, line, branchunless, lfinish[1]); NO_CHECK(COMPILE(ret, "defined/colon2#nd_head", node->nd_head)); @@ -4992,22 +4996,45 @@ defined_expr0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, https://github.com/ruby/ruby/blob/trunk/compile.c#L4996 (type == NODE_CALL || type == NODE_OPCALL || (type == NODE_ATTRASGN && !private_recv_p(node))); - if (!lfinish[1] && (node->nd_args || explicit_receiver)) { - lfinish[1] = NEW_LABEL(line); - } + if (node->nd_args || explicit_receiver) { + if (!lfinish[1]) { + lfinish[1] = NEW_LABEL(line); + } + if (!lfinish[2]) { + lfinish[2] = NEW_LABEL(line); + } + } if (node->nd_args) { - defined_expr0(iseq, ret, node->nd_args, lfinish, Qfalse); + defined_expr0(iseq, ret, node->nd_args, lfinish, Qfalse, false); ADD_INSNL(ret, line, branchunless, lfinish[1]); } if (explicit_receiver) { - defined_expr0(iseq, ret, node->nd_recv, lfinish, Qfalse); - ADD_INSNL(ret, line, branchunless, lfinish[1]); - NO_CHECK(COMPILE(ret, "defined/recv", node->nd_recv)); + defined_expr0(iseq, ret, node->nd_recv, lfinish, Qfalse, true); + switch(nd_type(node->nd_recv)) { + case NODE_CALL: + case NODE_OPCALL: + case NODE_VCALL: + case NODE_FCALL: + case NODE_ATTRASGN: + ADD_INSNL(ret, line, branchunless, lfinish[2]); + compile_call(iseq, ret, node->nd_recv, nd_type(node->nd_recv), line, 0, true); + break; + default: + ADD_INSNL(ret, line, branchunless, lfinish[1]); + NO_CHECK(COMPILE(ret, "defined/recv", node->nd_recv)); + break; + } + if (keep_result) { + ADD_INSN(ret, line, dup); + } ADD_INSN3(ret, line, defined, INT2FIX(DEFINED_METHOD), ID2SYM(node->nd_mid), PUSH_VAL(DEFINED_METHOD)); } else { ADD_INSN(ret, line, putself); + if (keep_result) { + ADD_INSN(ret, line, dup); + } ADD_INSN3(ret, line, defined, INT2FIX(DEFINED_FUNC), ID2SYM(node->nd_mid), PUSH_VAL(DEFINED_METHOD)); } @@ -5075,7 +5102,7 @@ defined_expr(rb_iseq_t *iseq, LINK_ANCHOR *const ret, https://github.com/ruby/ruby/blob/trunk/compile.c#L5102 const NODE *const node, LABEL **lfinish, VALUE needstr) { LINK_ELEMENT *lcur = ret->last; - defined_expr0(iseq, ret, node, lfinish, needstr); + defined_expr0(iseq, ret, node, lfinish, needstr, false); if (lfinish[1]) { int line = nd_line(node); LABEL *lstart = NEW_LABEL(line); @@ -5104,14 +5131,18 @@ compile_defined_expr(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const https://github.c (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/