[前][次][番号順一覧][スレッド一覧]

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/

[前][次][番号順一覧][スレッド一覧]