ruby-changes:69325
From: Alan <ko1@a...>
Date: Fri, 22 Oct 2021 04:07:50 +0900 (JST)
Subject: [ruby-changes:69325] bdfc23cba9 (master): YJIT: don't compile attr_accessor methods when tracing (#4998)
https://git.ruby-lang.org/ruby.git/commit/?id=bdfc23cba9 From bdfc23cba9c7ade8f4528f38b19b0ea11c0d56c4 Mon Sep 17 00:00:00 2001 From: Alan Wu <XrXr@u...> Date: Thu, 21 Oct 2021 15:07:32 -0400 Subject: YJIT: don't compile attr_accessor methods when tracing (#4998) 2d98593bf54a37397c6e4886ccc7e3654c2eaf85 made it so that attr_accessor methods fire C method tracing events. Previously, we weren't checking for whether we are tracing before compiling, leading to missed events. Since global invalidation invalidates all code, and that attr_accessor methods can never enable tracing while running, events are only dropped when YJIT tries to compile when tracing is already enabled. Factor out the code for checking tracing and check it before generating code for attr_accessor methods. This change fixes TestSetTraceFunc#test_tracepoint_attr when it's ran in isolation. --- bootstraptest/test_yjit.rb | 50 ++++++++++++++++++++++++++++++++++ yjit_codegen.c | 67 +++++++++++++++++++++++++++++++--------------- 2 files changed, 95 insertions(+), 22 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index a58ff2d18f..53cdcc69ac 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -2214,3 +2214,53 @@ assert_equal 'ok', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L2214 :ok } + +# regression test for tracing attr_accessor methods. +assert_equal "true", %q{ + c = Class.new do + attr_accessor :x + alias y x + alias y= x= + end + obj = c.new + + ar_meth = obj.method(:x) + aw_meth = obj.method(:x=) + aar_meth = obj.method(:y) + aaw_meth = obj.method(:y=) + events = [] + trace = TracePoint.new(:c_call, :c_return){|tp| + next if tp.path != __FILE__ + next if tp.method_id == :call + case tp.event + when :c_call + events << [tp.event, tp.method_id, tp.callee_id] + when :c_return + events << [tp.event, tp.method_id, tp.callee_id, tp.return_value] + end + } + test_proc = proc do + obj.x = 1 + obj.x + obj.y = 2 + obj.y + aw_meth.call(1) + ar_meth.call + aaw_meth.call(2) + aar_meth.call + end + test_proc.call # populate call caches + trace.enable(&test_proc) + expected = [ + [:c_call, :x=, :x=], + [:c_return, :x=, :x=, 1], + [:c_call, :x, :x], + [:c_return, :x, :x, 1], + [:c_call, :x=, :y=], + [:c_return, :x=, :y=, 2], + [:c_call, :x, :y], + [:c_return, :x, :y, 2], + ] * 2 + + expected == events +} diff --git a/yjit_codegen.c b/yjit_codegen.c index 3b39c043b5..be4d7f0165 100644 --- a/yjit_codegen.c +++ b/yjit_codegen.c @@ -3076,6 +3076,24 @@ lookup_cfunc_codegen(const rb_method_definition_t *def) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L3076 return NULL; } +// Is anyone listening for :c_call and :c_return event currently? +static bool +c_method_tracing_currently_enabled(const jitstate_t *jit) +{ + rb_event_flag_t tracing_events; + if (rb_multi_ractor_p()) { + tracing_events = ruby_vm_event_enabled_global_flags; + } + else { + // At the time of writing, events are never removed from + // ruby_vm_event_enabled_global_flags so always checking using it would + // mean we don't compile even after tracing is disabled. + tracing_events = rb_ec_ractor_hooks(jit->ec)->events; + } + + return tracing_events & (RUBY_EVENT_C_CALL | RUBY_EVENT_C_RETURN); +} + static codegen_status_t gen_send_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, rb_iseq_t *block, const int32_t argc, VALUE *recv_known_klass) { @@ -3099,23 +3117,10 @@ gen_send_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L3117 return YJIT_CANT_COMPILE; } - // Don't JIT if tracing c_call or c_return - { - rb_event_flag_t tracing_events; - if (rb_multi_ractor_p()) { - tracing_events = ruby_vm_event_enabled_global_flags; - } - else { - // We could always use ruby_vm_event_enabled_global_flags, - // but since events are never removed from it, doing so would mean - // we don't compile even after tracing is disabled. - tracing_events = rb_ec_ractor_hooks(jit->ec)->events; - } - - if (tracing_events & (RUBY_EVENT_C_CALL | RUBY_EVENT_C_RETURN)) { - GEN_COUNTER_INC(cb, send_cfunc_tracing); - return YJIT_CANT_COMPILE; - } + if (c_method_tracing_currently_enabled(jit)) { + // Don't JIT if tracing c_call or c_return + GEN_COUNTER_INC(cb, send_cfunc_tracing); + return YJIT_CANT_COMPILE; } // Delegate to codegen for C methods if we have it. @@ -3848,12 +3853,24 @@ gen_send_general(jitstate_t *jit, ctx_t *ctx, struct rb_call_data *cd, rb_iseq_t https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L3853 GEN_COUNTER_INC(cb, send_getter_arity); return YJIT_CANT_COMPILE; } - else { - mov(cb, REG0, recv); - - ID ivar_name = cme->def->body.attr.id; - return gen_get_ivar(jit, ctx, SEND_MAX_DEPTH, comptime_recv, ivar_name, recv_opnd, side_exit); + if (c_method_tracing_currently_enabled(jit)) { + // Can't generate code for firing c_call and c_return events + // :attr-tracing: + // Handling the C method tracing events for attr_accessor + // methods is easier than regular C methods as we know the + // "method" we are calling into never enables those tracing + // events. Once global invalidation runs, the code for the + // attr_accessor is invalidated and we exit at the closest + // instruction boundary which is always outside of the body of + // the attr_accessor code. + GEN_COUNTER_INC(cb, send_cfunc_tracing); + return YJIT_CANT_COMPILE; } + + mov(cb, REG0, recv); + + ID ivar_name = cme->def->body.attr.id; + return gen_get_ivar(jit, ctx, SEND_MAX_DEPTH, comptime_recv, ivar_name, recv_opnd, side_exit); case VM_METHOD_TYPE_ATTRSET: if ((vm_ci_flag(ci) & VM_CALL_KWARG) != 0) { GEN_COUNTER_INC(cb, send_attrset_kwargs); @@ -3863,6 +3880,12 @@ gen_send_general(jitstate_t *jit, ctx_t *ctx, struct rb_call_data *cd, rb_iseq_t https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L3880 GEN_COUNTER_INC(cb, send_ivar_set_method); return YJIT_CANT_COMPILE; } + else if (c_method_tracing_currently_enabled(jit)) { + // Can't generate code for firing c_call and c_return events + // See :attr-tracing: + GEN_COUNTER_INC(cb, send_cfunc_tracing); + return YJIT_CANT_COMPILE; + } else { ID ivar_name = cme->def->body.attr.id; return gen_set_ivar(jit, ctx, comptime_recv, comptime_recv_klass, ivar_name); -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/