ruby-changes:50507
From: k0kubun <ko1@a...>
Date: Sat, 3 Mar 2018 13:07:08 +0900 (JST)
Subject: [ruby-changes:50507] k0kubun:r62641 (trunk): vm.c: add mjit_enable_p flag
k0kubun 2018-03-03 13:07:02 +0900 (Sat, 03 Mar 2018) New Revision: 62641 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=62641 Log: vm.c: add mjit_enable_p flag to count up total calls properly. Some places (especially CALL_METHOD) invoke mjit_exec twice for one method call. It would be problematic when debugging, or possibly it would result in a wrong profiling result. This commit doesn't have impact for performance: * Optcarrot benchmark ** before fps: 59.37757770848619 fps: 56.49998488958699 fps: 59.07900362739362 fps: 58.924749807695996 fps: 57.667905665594894 fps: 57.540021018385254 fps: 59.5518055679647 fps: 55.93831555148311 fps: 57.82685112863262 fps: 59.22391754481736 checksum: 59662 ** after fps: 58.461881158098194 fps: 59.32685183081354 fps: 54.11334310279802 fps: 59.2281560439788 fps: 58.60495705318312 fps: 55.696478648491045 fps: 58.49003452654724 fps: 58.387771929393224 fps: 59.24156772816439 fps: 56.68804731968107 checksum: 59662 * Discourse Your Results: (note for timings- percentile is first, duration is second in millisecs) ** before (without JIT) categories_admin: 50: 16 75: 17 90: 24 99: 37 home_admin: 50: 20 75: 20 90: 24 99: 42 topic_admin: 50: 16 75: 16 90: 18 99: 28 categories: 50: 36 75: 37 90: 45 99: 68 home: 50: 38 75: 40 90: 53 99: 92 topic: 50: 14 75: 15 90: 17 99: 26 ** after (without JIT) categories_admin: 50: 16 75: 16 90: 24 99: 36 home_admin: 50: 19 75: 20 90: 23 99: 41 topic_admin: 50: 16 75: 16 90: 19 99: 33 categories: 50: 35 75: 36 90: 44 99: 61 home: 50: 38 75: 40 90: 52 99: 101 topic: 50: 14 75: 15 90: 15 99: 24 ** before (with JIT) categories_admin: 50: 19 75: 23 90: 29 99: 44 home_admin: 50: 24 75: 26 90: 32 99: 46 topic_admin: 50: 20 75: 22 90: 27 99: 44 categories: 50: 41 75: 43 90: 51 99: 66 home: 50: 46 75: 49 90: 56 99: 68 topic: 50: 18 75: 19 90: 22 99: 31 ** after (with JIT) categories_admin: 50: 18 75: 21 90: 28 99: 42 home_admin: 50: 23 75: 25 90: 31 99: 51 topic_admin: 50: 19 75: 20 90: 24 99: 31 categories: 50: 41 75: 44 90: 52 99: 69 home: 50: 45 75: 48 90: 61 99: 88 topic: 50: 19 75: 20 90: 24 99: 33 Modified files: trunk/insns.def trunk/tool/ruby_vm/views/_mjit_compile_send.erb trunk/tool/ruby_vm/views/mjit_compile.inc.erb trunk/vm.c trunk/vm_eval.c trunk/vm_exec.h trunk/vm_insnhelper.h Index: vm.c =================================================================== --- vm.c (revision 62640) +++ vm.c (revision 62641) @@ -1006,7 +1006,7 @@ invoke_block(rb_execution_context_t *ec, https://github.com/ruby/ruby/blob/trunk/vm.c#L1006 ec->cfp->sp + arg_size, iseq->body->local_table_size - arg_size, iseq->body->stack_max); - return vm_exec(ec); + return vm_exec(ec, TRUE); } static VALUE @@ -1027,7 +1027,7 @@ invoke_bmethod(rb_execution_context_t *e https://github.com/ruby/ruby/blob/trunk/vm.c#L1027 RUBY_DTRACE_METHOD_ENTRY_HOOK(ec, me->owner, me->def->original_id); EXEC_EVENT_HOOK(ec, RUBY_EVENT_CALL, self, me->def->original_id, me->called_id, me->owner, Qnil); VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_FINISH); - ret = vm_exec(ec); + ret = vm_exec(ec, TRUE); EXEC_EVENT_HOOK(ec, RUBY_EVENT_RETURN, self, me->def->original_id, me->called_id, me->owner, ret); RUBY_DTRACE_METHOD_RETURN_HOOK(ec, me->owner, me->def->original_id); return ret; @@ -1788,13 +1788,16 @@ hook_before_rewind(rb_execution_context_ https://github.com/ruby/ruby/blob/trunk/vm.c#L1788 VALUE *ep; // ep void *code; // }; + + If mjit_exec is already called before calling vm_exec, `mjit_enable_p` should + be FALSE to avoid calling `mjit_exec` twice. */ MJIT_FUNC_EXPORTED VALUE -vm_exec(rb_execution_context_t *ec) +vm_exec(rb_execution_context_t *ec, int mjit_enable_p) { enum ruby_tag_type state; - VALUE result; + VALUE result = Qundef; VALUE initial = 0; struct vm_throw_data *err; @@ -1802,7 +1805,8 @@ vm_exec(rb_execution_context_t *ec) https://github.com/ruby/ruby/blob/trunk/vm.c#L1805 _tag.retval = Qnil; if ((state = EC_EXEC_TAG()) == TAG_NONE) { - result = mjit_exec(ec); + if (mjit_enable_p) + result = mjit_exec(ec); vm_loop_start: if (result == Qundef) result = vm_exec_core(ec, initial); @@ -2047,7 +2051,7 @@ rb_iseq_eval(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/vm.c#L2051 rb_execution_context_t *ec = GET_EC(); VALUE val; vm_set_top_stack(ec, iseq); - val = vm_exec(ec); + val = vm_exec(ec, TRUE); return val; } @@ -2058,7 +2062,7 @@ rb_iseq_eval_main(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/vm.c#L2062 VALUE val; vm_set_main_stack(ec, iseq); - val = vm_exec(ec); + val = vm_exec(ec, TRUE); return val; } Index: tool/ruby_vm/views/mjit_compile.inc.erb =================================================================== --- tool/ruby_vm/views/mjit_compile.inc.erb (revision 62640) +++ tool/ruby_vm/views/mjit_compile.inc.erb (revision 62641) @@ -37,7 +37,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/mjit_compile.inc.erb#L37 % # PUSH(): refers to `SET_SV()`, `INC_SP()` % # GET_SELF(): refers to `reg_cfp->self` % # GET_LEP(): refers to `VM_EP_LEP(reg_cfp->ep)` -% # EXEC_EC_CFP(): refers to `val = vm_exec(ec)` with frame setup +% # EXEC_EC_CFP(): refers to `val = vm_exec(ec, TRUE)` with frame setup % # CALL_METHOD(): using `GET_CFP()` and `EXEC_EC_CFP()` % # TOPN(): refers to `reg_cfp->sp`, which needs to have correct sp (of course) % # STACK_ADDR_FROM_TOP(): refers to `reg_cfp->sp`, same problem here Index: tool/ruby_vm/views/_mjit_compile_send.erb =================================================================== --- tool/ruby_vm/views/_mjit_compile_send.erb (revision 62640) +++ tool/ruby_vm/views/_mjit_compile_send.erb (revision 62641) @@ -54,7 +54,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_send.erb#L54 (VALUE)iseq, (VALUE)cc->me, (VALUE)iseq->body->iseq_encoded, param_size, iseq->body->local_table_size - param_size, iseq->body->stack_max); fprintf(f, " if ((v = mjit_exec(ec)) == Qundef) {\n"); fprintf(f, " VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_FINISH);\n"); /* This is vm_call0_body's code after vm_call_iseq_setup */ - fprintf(f, " v = vm_exec(ec);\n"); + fprintf(f, " v = vm_exec(ec, FALSE);\n"); fprintf(f, " }\n"); fprintf(f, " stack[%d] = v;\n", b->stack_size - argc - 1); fprintf(f, " }\n"); Index: insns.def =================================================================== --- insns.def (revision 62640) +++ insns.def (revision 62641) @@ -693,7 +693,7 @@ defineclass https://github.com/ruby/ruby/blob/trunk/insns.def#L693 class_iseq->body->iseq_encoded, GET_SP(), class_iseq->body->local_table_size, class_iseq->body->stack_max); - EXEC_EC_CFP(); + EXEC_EC_CFP(TRUE); } /**********************************************************/ @@ -820,7 +820,7 @@ invokeblock https://github.com/ruby/ruby/blob/trunk/insns.def#L820 val = vm_invoke_block(ec, GET_CFP(), &calling, ci, block_handler); if (val == Qundef) { - EXEC_EC_CFP(); + EXEC_EC_CFP(TRUE); } } @@ -1344,7 +1344,7 @@ opt_call_c_function https://github.com/ruby/ruby/blob/trunk/insns.def#L1344 THROW_EXCEPTION(err); } - EXEC_EC_CFP(); + EXEC_EC_CFP(TRUE); } /* BLT */ Index: vm_insnhelper.h =================================================================== --- vm_insnhelper.h (revision 62640) +++ vm_insnhelper.h (revision 62641) @@ -130,7 +130,7 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L130 #define CALL_METHOD(calling, ci, cc) do { \ VALUE v = (*(cc)->call)(ec, GET_CFP(), (calling), (ci), (cc)); \ if (v == Qundef && (v = mjit_exec(ec)) == Qundef) { \ - EXEC_EC_CFP(); \ + EXEC_EC_CFP(FALSE); \ } \ else { \ val = v; \ Index: vm_eval.c =================================================================== --- vm_eval.c (revision 62640) +++ vm_eval.c (revision 62641) @@ -20,7 +20,7 @@ static inline VALUE vm_yield_with_cref(r https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L20 static inline VALUE vm_yield(rb_execution_context_t *ec, int argc, const VALUE *argv); static inline VALUE vm_yield_with_block(rb_execution_context_t *ec, int argc, const VALUE *argv, VALUE block_handler); static inline VALUE vm_yield_force_blockarg(rb_execution_context_t *ec, VALUE args); -VALUE vm_exec(rb_execution_context_t *ec); +VALUE vm_exec(rb_execution_context_t *ec, int mjit_enable_p); static void vm_set_eval_stack(rb_execution_context_t * th, const rb_iseq_t *iseq, const rb_cref_t *cref, const struct rb_block *base_block); static int vm_collect_local_variables_in_heap(const VALUE *dfp, const struct local_var_list *vars); @@ -126,7 +126,7 @@ vm_call0_body(rb_execution_context_t *ec https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L126 vm_call_iseq_setup(ec, reg_cfp, calling, ci, cc); VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_FINISH); - return vm_exec(ec); /* CHECK_INTS in this function */ + return vm_exec(ec, TRUE); /* CHECK_INTS in this function */ } case VM_METHOD_TYPE_NOTIMPLEMENTED: case VM_METHOD_TYPE_CFUNC: @@ -1327,7 +1327,7 @@ eval_string_with_cref(VALUE self, VALUE https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L1327 vm_set_eval_stack(ec, iseq, cref, &block); /* kick */ - return vm_exec(ec); + return vm_exec(ec, TRUE); } static VALUE @@ -1348,7 +1348,7 @@ eval_string_with_scope(VALUE scope, VALU https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L1348 } /* kick */ - return vm_exec(ec); + return vm_exec(ec, TRUE); } /* Index: vm_exec.h =================================================================== --- vm_exec.h (revision 62640) +++ vm_exec.h (revision 62641) @@ -168,12 +168,12 @@ default: \ https://github.com/ruby/ruby/blob/trunk/vm_exec.h#L168 #endif #ifdef MJIT_HEADER -#define EXEC_EC_CFP() do { \ +#define EXEC_EC_CFP(mjit_enable_p) do { \ VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_FINISH); \ - val = vm_exec(ec); \ + val = vm_exec(ec, mjit_enable_p); \ } while (0) #else -#define EXEC_EC_CFP() do { \ +#define EXEC_EC_CFP(mjit_enable_p) do { \ RESTORE_REGS(); \ NEXT_INSN(); \ } while (0) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/