ruby-changes:62562
From: Jeremy <ko1@a...>
Date: Thu, 13 Aug 2020 03:44:32 +0900 (JST)
Subject: [ruby-changes:62562] 4fc6cfbeae (master): Revert "Improve performance of partial backtraces"
https://git.ruby-lang.org/ruby.git/commit/?id=4fc6cfbeae From 4fc6cfbeae3c86e8f3675c70b417356ecd3d4a56 Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Wed, 12 Aug 2020 11:43:11 -0700 Subject: Revert "Improve performance of partial backtraces" This reverts commit f2d7461e85053cb084e10999b0b8019b0c29e66e. Some CI machines are reporting issues with backtrace_mark, so I'm going to revert this for now. diff --git a/vm_backtrace.c b/vm_backtrace.c index 50cc574..04b696c 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -29,9 +29,6 @@ id2str(ID id) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L29 } #define rb_id2str(id) id2str(id) -#define BACKTRACE_START 0 -#define ALL_BACKTRACE_LINES -1 - inline static int calc_lineno(const rb_iseq_t *iseq, const VALUE *pc) { @@ -129,10 +126,6 @@ location_mark_entry(rb_backtrace_location_t *fi) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L126 rb_gc_mark_movable((VALUE)fi->body.iseq.iseq); break; case LOCATION_TYPE_CFUNC: - if (fi->body.cfunc.prev_loc) { - location_mark_entry(fi->body.cfunc.prev_loc); - } - break; case LOCATION_TYPE_IFUNC: default: break; @@ -491,47 +484,22 @@ backtrace_alloc(VALUE klass) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L484 return obj; } -static long -backtrace_size(const rb_execution_context_t *ec) -{ - const rb_control_frame_t *last_cfp = ec->cfp; - const rb_control_frame_t *start_cfp = RUBY_VM_END_CONTROL_FRAME(ec); - - if (start_cfp == NULL) { - return -1; - } - - start_cfp = - RUBY_VM_NEXT_CONTROL_FRAME( - RUBY_VM_NEXT_CONTROL_FRAME(start_cfp)); /* skip top frames */ - - if (start_cfp < last_cfp) { - return 0; - } - - return start_cfp - last_cfp + 1; -} - -static int +static void backtrace_each(const rb_execution_context_t *ec, - ptrdiff_t from_last, - long num_frames, void (*init)(void *arg, size_t size), void (*iter_iseq)(void *arg, const rb_control_frame_t *cfp), void (*iter_cfunc)(void *arg, const rb_control_frame_t *cfp, ID mid), - void (*iter_skip)(void *arg, const rb_control_frame_t *cfp), void *arg) { const rb_control_frame_t *last_cfp = ec->cfp; const rb_control_frame_t *start_cfp = RUBY_VM_END_CONTROL_FRAME(ec); const rb_control_frame_t *cfp; - ptrdiff_t size, i, last, start = 0; - int ret = 0; + ptrdiff_t size, i; // In the case the thread vm_stack or cfp is not initialized, there is no backtrace. if (start_cfp == NULL) { init(arg, 0); - return ret; + return; } /* <- start_cfp (end control frame) @@ -549,43 +517,16 @@ backtrace_each(const rb_execution_context_t *ec, https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L517 RUBY_VM_NEXT_CONTROL_FRAME(start_cfp)); /* skip top frames */ if (start_cfp < last_cfp) { - size = last = 0; + size = 0; } else { size = start_cfp - last_cfp + 1; - - if (from_last > size) { - size = last = 0; - ret = 1; - } - else if (num_frames >= 0 && num_frames < size) { - if (from_last + num_frames > size) { - size -= from_last; - last = size; - } - else { - start = size - from_last - num_frames; - size = num_frames; - last = start + size; - } - } - else { - size -= from_last; - last = size; - } } init(arg, size); /* SDR(); */ - for (i=0, cfp = start_cfp; i<last; i++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) { - if (i < start) { - if (iter_skip) { - iter_skip(arg, cfp); - } - continue; - } - + for (i=0, cfp = start_cfp; i<size; i++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) { /* fprintf(stderr, "cfp: %d\n", (rb_control_frame_t *)(ec->vm_stack + ec->vm_stack_size) - cfp); */ if (cfp->iseq) { if (cfp->pc) { @@ -599,16 +540,12 @@ backtrace_each(const rb_execution_context_t *ec, https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L540 iter_cfunc(arg, cfp, mid); } } - - return ret; } struct bt_iter_arg { rb_backtrace_t *bt; VALUE btobj; rb_backtrace_location_t *prev_loc; - const rb_control_frame_t *prev_cfp; - rb_backtrace_location_t *init_loc; }; static void @@ -617,11 +554,8 @@ bt_init(void *ptr, size_t size) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L554 struct bt_iter_arg *arg = (struct bt_iter_arg *)ptr; arg->btobj = backtrace_alloc(rb_cBacktrace); GetCoreDataFromValue(arg->btobj, rb_backtrace_t, arg->bt); - arg->bt->backtrace_base = arg->bt->backtrace = ALLOC_N(rb_backtrace_location_t, size+1); - arg->bt->backtrace_size = 1; - arg->prev_cfp = NULL; - arg->init_loc = &arg->bt->backtrace_base[size]; - arg->init_loc->type = 0; + arg->bt->backtrace_base = arg->bt->backtrace = ALLOC_N(rb_backtrace_location_t, size); + arg->bt->backtrace_size = 0; } static void @@ -630,7 +564,7 @@ bt_iter_iseq(void *ptr, const rb_control_frame_t *cfp) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L564 const rb_iseq_t *iseq = cfp->iseq; const VALUE *pc = cfp->pc; struct bt_iter_arg *arg = (struct bt_iter_arg *)ptr; - rb_backtrace_location_t *loc = &arg->bt->backtrace[arg->bt->backtrace_size++-1]; + rb_backtrace_location_t *loc = &arg->bt->backtrace[arg->bt->backtrace_size++]; loc->type = LOCATION_TYPE_ISEQ; loc->body.iseq.iseq = iseq; loc->body.iseq.lineno.pc = pc; @@ -641,69 +575,41 @@ static void https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L575 bt_iter_cfunc(void *ptr, const rb_control_frame_t *cfp, ID mid) { struct bt_iter_arg *arg = (struct bt_iter_arg *)ptr; - rb_backtrace_location_t *loc = &arg->bt->backtrace[arg->bt->backtrace_size++-1]; + rb_backtrace_location_t *loc = &arg->bt->backtrace[arg->bt->backtrace_size++]; loc->type = LOCATION_TYPE_CFUNC; loc->body.cfunc.mid = mid; - if (arg->prev_loc) { - loc->body.cfunc.prev_loc = arg->prev_loc; - } - else if (arg->prev_cfp) { - const rb_iseq_t *iseq = arg->prev_cfp->iseq; - const VALUE *pc = arg->prev_cfp->pc; - arg->init_loc->type = LOCATION_TYPE_ISEQ; - arg->init_loc->body.iseq.iseq = iseq; - arg->init_loc->body.iseq.lineno.pc = pc; - loc->body.cfunc.prev_loc = arg->prev_loc = arg->init_loc; - } else { - loc->body.cfunc.prev_loc = NULL; - } -} - -static void -bt_iter_skip(void *ptr, const rb_control_frame_t *cfp) -{ - if (cfp->iseq && cfp->pc) { - ((struct bt_iter_arg *)ptr)->prev_cfp = cfp; - } + loc->body.cfunc.prev_loc = arg->prev_loc; } -static VALUE -rb_ec_partial_backtrace_object(const rb_execution_context_t *ec, long lev, long n, int* level_too_large) +MJIT_FUNC_EXPORTED VALUE +rb_ec_backtrace_object(const rb_execution_context_t *ec) { struct bt_iter_arg arg; - int too_large; arg.prev_loc = 0; - too_large = backtrace_each(ec, - lev, - n, - bt_init, - bt_iter_iseq, - bt_iter_cfunc, - bt_iter_skip, - &arg); - - if (level_too_large) *level_too_large = too_large; + backtrace_each(ec, + bt_init, + bt_iter_iseq, + bt_iter_cfunc, + &arg); return arg.btobj; } -MJIT_FUNC_EXPORTED VALUE -rb_ec_backtrace_object(const rb_execution_context_t *ec) -{ - return rb_ec_partial_backtrace_object(ec, BACKTRACE_START, ALL_BACKTRACE_LINES, NULL); -} - static VALUE -backtrace_collect(rb_backtrace_t *bt, VALUE (*func)(rb_backtrace_location_t *, void *arg), void *arg) +backtrace_collect(rb_backtrace_t *bt, long lev, long n, VALUE (*func)(rb_backtrace_location_t *, void *arg), void *arg) { VALUE btary; int i; - btary = rb_ary_new2(bt->backtrace_size-1); + if (UNLIKELY(lev < 0 || n < 0)) { + rb_bug("backtrace_collect: unreachable"); + } - for (i=0; i<bt->backtrace_size-1; i++) { - rb_backtrace_location_t *loc = &bt->backtrace[bt->backtrace_size - 2 - i]; + btary = rb_ary_new2(n); + + for (i=0; i+lev<bt->backtrace_size && i<n; i++) { + rb_backtrace_location_t *loc = &bt->backtrace[bt->backtrace_size - 1 - (lev+i)]; rb_ary_push(btary, func(loc, arg)); } @@ -717,12 +623,23 @@ location_to_str_dmyarg(rb_backtrace_location_t *loc, void *dmy) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L623 } static VALUE -backtrace_to_str_ary(VALUE self) +backtrace_to_str_ary(VALUE self, long lev, long n) { - VALUE r; rb_backtrace_t *bt; + int size; + VALUE r; + GetCoreDataFromValue(self, rb_backtrace_t, bt); - r = backtrace_collect(bt, location_to_str_dmyarg, 0); + size = bt->backtrace_size; + + if (n == 0) { + n = size; + } + if (lev > size) { + return Qnil; + } + + r = backtrace_collect(bt, lev, n, location_to_str_dmyarg, 0); RB_GC_GUARD(self); return r; } @@ -734,7 +651,7 @@ rb_backtrace_to_str_ary(VALUE self) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L651 GetCoreDataFromValue(self, rb_backtrace_t, bt); if (!bt->strary) { - bt->strary = backtrace_to_str_ary(self); + bt->strary = backtrace_to_str_ary(self, 0, bt->backtrace_size); } return bt->strary; } @@ -747,9 +664,9 @@ rb_backtrace_use_iseq_first_lineno_for_last_location(VALUE self) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L664 rb_backtrace_location_t *loc; GetCoreDataFromValue(self, rb_backtrace_t, bt); - VM_ASSERT(bt->backtrace_size > 1); + VM_ASSERT(bt->backtrace_size > 0); - loc = &bt->backtrace[bt->backtrace_size - 2]; + loc = & (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/