ko1	2011-08-24 15:31:15 +0900 (Wed, 24 Aug 2011)

  New Revision: 33046

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=33046

  Log:
    * iseq.h, iseq.c, compile.c: Change the line number data structure
      to solve an issue reported at [ruby-dev:44413] [Ruby 1.9 - Bug #5217].
      Before this fix, each instruction has an information including
      line number (iseq::iseq_insn_info_table).  Instead of this data
      structure, recording only line number changing places
      (iseq::iseq_line_info_table).
      The order of entries in iseq_line_info_table is ascending order of
      iseq_line_info_table_entry::position.  You can get a line number
      by an iseq and a program counter with this data structure.
      This fix reduces memory consumption of iseq (bytecode).
      On my measurement, a rails application consumes 21.8MB for
      iseq with this fix on the 32bit CPU.  Without this fix, it
      consumes 24.7MB for iseq [ruby-dev:44415].
    * proc.c: ditto.
    * vm_insnhelper.c: ditto.
    * vm_method.c: ditto.
    * vm.c (rb_vm_get_sourceline): change to use rb_iseq_line_no().

  Modified files:
    trunk/ChangeLog
    trunk/compile.c
    trunk/iseq.c
    trunk/iseq.h
    trunk/proc.c
    trunk/vm.c
    trunk/vm_core.h
    trunk/vm_insnhelper.c
    trunk/vm_method.c

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 33045)
+++ ChangeLog	(revision 33046)
@@ -1,3 +1,27 @@
+Wed Aug 24 15:13:56 2011  Koichi Sasada  <ko1@a...>
+
+	* iseq.h, iseq.c, compile.c: Change the line number data structure
+	  to solve an issue reported at [ruby-dev:44413] [Ruby 1.9 - Bug #5217].
+	  Before this fix, each instruction has an information including
+	  line number (iseq::iseq_insn_info_table).  Instead of this data
+	  structure, recording only line number changing places
+	  (iseq::iseq_line_info_table).
+	  The order of entries in iseq_line_info_table is ascending order of
+	  iseq_line_info_table_entry::position.  You can get a line number
+	  by an iseq and a program counter with this data structure.
+	  This fix reduces memory consumption of iseq (bytecode).
+	  On my measurement, a rails application consumes 21.8MB for
+	  iseq with this fix on the 32bit CPU.  Without this fix, it
+	  consumes 24.7MB for iseq [ruby-dev:44415].
+
+	* proc.c: ditto.
+
+	* vm_insnhelper.c: ditto.
+
+	* vm_method.c: ditto.
+
+	* vm.c (rb_vm_get_sourceline): change to use rb_iseq_line_no().
+
 Wed Aug 24 09:49:10 2011  Koichi Sasada  <ko1@a...>
 
 	* insns.def (defined): fix to checking class variable.
Index: vm_core.h
===================================================================
--- vm_core.h	(revision 33045)
+++ vm_core.h	(revision 33046)
@@ -176,8 +176,8 @@
     unsigned short line_no;
 
     /* insn info, must be freed */
-    struct iseq_insn_info_entry *insn_info_table;
-    size_t insn_info_size;
+    struct iseq_line_info_entry *line_info_table;
+    size_t line_info_size;
 
     ID *local_table;		/* must free */
     int local_table_size;
Index: iseq.c
===================================================================
--- iseq.c	(revision 33045)
+++ iseq.c	(revision 33046)
@@ -78,7 +78,7 @@
 	    }
 
 	    RUBY_FREE_UNLESS_NULL(iseq->iseq);
-	    RUBY_FREE_UNLESS_NULL(iseq->insn_info_table);
+	    RUBY_FREE_UNLESS_NULL(iseq->line_info_table);
 	    RUBY_FREE_UNLESS_NULL(iseq->local_table);
 	    RUBY_FREE_UNLESS_NULL(iseq->ic_entries);
 	    RUBY_FREE_UNLESS_NULL(iseq->catch_table);
@@ -136,7 +136,7 @@
 	    }
 
 	    size += iseq->iseq_size * sizeof(VALUE);
-	    size += iseq->insn_info_size * sizeof(struct iseq_insn_info_entry);
+	    size += iseq->line_info_size * sizeof(struct iseq_line_info_entry);
 	    size += iseq->local_table_size * sizeof(ID);
 	    size += iseq->catch_table_size * sizeof(struct iseq_catch_table_entry);
 	    size += iseq->arg_opts * sizeof(VALUE);
@@ -680,25 +680,45 @@
 /* TODO: search algorithm is brute force.
          this should be binary search or so. */
 
-static struct iseq_insn_info_entry *
-get_insn_info(const rb_iseq_t *iseq, const unsigned long pos)
+static struct iseq_line_info_entry *
+get_line_info(const rb_iseq_t *iseq, size_t pos)
 {
-    unsigned long i, size = iseq->insn_info_size;
-    struct iseq_insn_info_entry *table = iseq->insn_info_table;
+    size_t i = 0, size = iseq->line_info_size;
+    struct iseq_line_info_entry *table = iseq->line_info_table;
+    const int debug = 0;
 
-    for (i = 0; i < size; i++) {
+    if (debug) {
+	printf("size: %d\n", size);
+	printf("table[%d]: position: %d, line: %d, pos: %d\n",
+	       i, table[i].position, table[i].line_no, pos);
+    }
+
+    if (size == 0) {
+	return 0;
+    }
+    else if (size == 1) {
+	return &table[0];
+    }
+    else {
+	for (i=1; i<size; i++) {
+	    if (debug) printf("table[%d]: position: %d, line: %d, pos: %d\n",
+			      i, table[i].position, table[i].line_no, pos);
+
 	if (table[i].position == pos) {
 	    return &table[i];
 	}
+	    if (table[i].position > pos) {
+		return &table[i-1];
     }
-
-    return 0;
+	}
+    }
+    return &table[i-1];
 }
 
-static unsigned short
-find_line_no(rb_iseq_t *iseq, unsigned long pos)
+static unsigned int
+find_line_no(const rb_iseq_t *iseq, size_t pos)
 {
-    struct iseq_insn_info_entry *entry = get_insn_info(iseq, pos);
+    struct iseq_line_info_entry *entry = get_line_info(iseq, pos);
     if (entry) {
 	return entry->line_no;
     }
@@ -707,26 +727,17 @@
     }
 }
 
-static unsigned short
-find_prev_line_no(rb_iseq_t *iseqdat, unsigned long pos)
+unsigned int
+rb_iseq_line_no(const rb_iseq_t *iseq, size_t pos)
 {
-    unsigned long i, size = iseqdat->insn_info_size;
-    struct iseq_insn_info_entry *iiary = iseqdat->insn_info_table;
-
-    for (i = 0; i < size; i++) {
-	if (iiary[i].position == pos) {
-	    if (i > 0) {
-		return iiary[i - 1].line_no;
+    if (pos == 0) {
+	return find_line_no(iseq, pos);
 	    }
 	    else {
-		return 0;
-	    }
+	return find_line_no(iseq, pos - 1);
 	}
     }
 
-    return 0;
-}
-
 static VALUE
 insn_operand_intern(rb_iseq_t *iseq,
 		    VALUE insn, int op_no, VALUE op,
@@ -865,23 +876,15 @@
 	}
     }
 
-    if (1) {
-	int line_no = find_line_no(iseqdat, pos);
-	int prev = find_prev_line_no(iseqdat, pos);
+    {
+	unsigned int line_no = find_line_no(iseqdat, pos);
+	unsigned int prev = pos == 0 ? 0 : find_line_no(iseqdat, pos - 1);
 	if (line_no && line_no != prev) {
 	    long slen = RSTRING_LEN(str);
 	    slen = (slen > 70) ? 0 : (70 - slen);
 	    str = rb_str_catf(str, "%*s(%4d)", (int)slen, "", line_no);
 	}
     }
-    else {
-	/* for debug */
-	struct iseq_insn_info_entry *entry = get_insn_info(iseqdat, pos);
-	long slen = RSTRING_LEN(str);
-	slen = (slen > 60) ? 0 : (60 - slen);
-	str = rb_str_catf(str, "%*s(line: %d, sp: %d)",
-			  (int)slen, "", entry->line_no, entry->sp);
-    }
 
     if (ret) {
 	rb_str_cat2(str, "\n");
@@ -1098,8 +1101,8 @@
 static VALUE
 iseq_data_to_ary(rb_iseq_t *iseq)
 {
-    long i, pos;
-    int line = 0;
+    long i, pos, ti;
+    unsigned int line = 0;
     VALUE *seq;
 
     VALUE val = rb_ary_new();
@@ -1298,6 +1301,7 @@
 
     /* make body with labels and insert line number */
     body = rb_ary_new();
+    ti = 0;
 
     for (i=0, pos=0; i<RARRAY_LEN(nbody); i++) {
 	VALUE ary = RARRAY_PTR(nbody)[i];
@@ -1307,9 +1311,10 @@
 	    rb_ary_push(body, (VALUE)label);
 	}
 
-	if (iseq->insn_info_table[i].line_no != line) {
-	    line = iseq->insn_info_table[i].line_no;
+	if (iseq->line_info_table[ti].position == pos) {
+	    line = iseq->line_info_table[ti].line_no;
 	    rb_ary_push(body, INT2FIX(line));
+	    ti++;
 	}
 
 	rb_ary_push(body, ary);
@@ -1441,7 +1446,7 @@
 rb_iseq_build_for_ruby2cext(
     const rb_iseq_t *iseq_template,
     const rb_insn_func_t *func,
-    const struct iseq_insn_info_entry *insn_info_table,
+    const struct iseq_line_info_entry *line_info_table,
     const char **local_table,
     const VALUE *arg_opt_table,
     const struct iseq_catch_table_entry *catch_table,
@@ -1479,8 +1484,8 @@
   } \
 } while (0)
 
-    ALLOC_AND_COPY(iseq->insn_info_table, insn_info_table,
-		   struct iseq_insn_info_entry, iseq->insn_info_size);
+    ALLOC_AND_COPY(iseq->line_info_table, line_info_table,
+		   struct iseq_line_info_entry, iseq->line_info_size);
 
     ALLOC_AND_COPY(iseq->catch_table, catch_table,
 		   struct iseq_catch_table_entry, iseq->catch_table_size);
Index: iseq.h
===================================================================
--- iseq.h	(revision 33045)
+++ iseq.h	(revision 33046)
@@ -26,6 +26,7 @@
 VALUE rb_iseq_load(VALUE data, VALUE parent, VALUE opt);
 VALUE rb_iseq_parameters(const rb_iseq_t *iseq, int is_proc);
 struct st_table *ruby_insn_make_insn_table(void);
+unsigned int rb_iseq_line_no(const rb_iseq_t *iseq, size_t pos);
 
 /* proc.c */
 rb_iseq_t *rb_method_get_iseq(VALUE body);
@@ -43,10 +44,9 @@
     int debug_level;
 };
 
-struct iseq_insn_info_entry {
-    unsigned short position;
-    unsigned short line_no;
-    unsigned short sp;
+struct iseq_line_info_entry {
+    unsigned int position;
+    unsigned int line_no;
 };
 
 struct iseq_catch_table_entry {
Index: compile.c
===================================================================
--- compile.c	(revision 33045)
+++ compile.c	(revision 33046)
@@ -51,7 +51,7 @@
 typedef struct iseq_insn_data {
     LINK_ELEMENT link;
     enum ruby_vminsn_type insn_id;
-    int line_no;
+    unsigned int line_no;
     int operand_size;
     int sc_state;
     VALUE *operands;
@@ -1283,7 +1283,8 @@
 {
     LABEL *lobj;
     INSN *iobj;
-    struct iseq_insn_info_entry *insn_info_table;
+    struct iseq_line_info_entry *line_info_table;
+    unsigned int last_line = 0;
     LINK_ELEMENT *list;
     VALUE *generated_iseq;
 
@@ -1335,7 +1336,7 @@
 
     /* make instruction sequence */
     generated_iseq = ALLOC_N(VALUE, pos);
-    insn_info_table = ALLOC_N(struct iseq_insn_info_entry, k);
+    line_info_table = ALLOC_N(struct iseq_line_info_entry, k);
     iseq->ic_entries = ALLOC_N(struct iseq_inline_cache_entry, iseq->ic_size);
     MEMZERO(iseq->ic_entries, struct iseq_inline_cache_entry, iseq->ic_size);
 
@@ -1373,7 +1374,7 @@
 				     "operand size miss! (%d for %d)",
 				     iobj->operand_size, len - 1);
 		    xfree(generated_iseq);
-		    xfree(insn_info_table);
+		    xfree(line_info_table);
 		    return 0;
 		}
 
@@ -1476,15 +1477,16 @@
 			rb_compile_error(RSTRING_PTR(iseq->filename), iobj->line_no,
 					 "unknown operand type: %c", type);
 			xfree(generated_iseq);
-			xfree(insn_info_table);
+			xfree(line_info_table);
 			return 0;
 		    }
 		}
-		insn_info_table[k].line_no = iobj->line_no;
-		insn_info_table[k].position = pos;
-		insn_info_table[k].sp = sp;
+		if (last_line != iobj->line_no) {
+		    line_info_table[k].line_no = last_line = iobj->line_no;
+		    line_info_table[k].position = pos;
+		k++;
+		}
 		pos += len;
-		k++;
 		break;
 	    }
 	  case ISEQ_ELEMENT_LABEL:
@@ -1512,19 +1514,21 @@
 
 		if (adjust->line_no != -1) {
 		    if (orig_sp - sp > 0) {
-			insn_info_table[k].line_no = adjust->line_no;
-			insn_info_table[k].position = pos;
-			insn_info_table[k].sp = sp;
+			if (last_line != (unsigned int)adjust->line_no) {
+			    line_info_table[k].line_no = last_line = adjust->line_no;
+			    line_info_table[k].position = pos;
 			k++;
+			}
 			generated_iseq[pos++] = BIN(adjuststack);
 			generated_iseq[pos++] = orig_sp - sp;
 		    }
 		    else if (orig_sp - sp == 0) {
 			/* jump to next insn */
-			insn_info_table[k].line_no = adjust->line_no;
-			insn_info_table[k].position = pos;
-			insn_info_table[k].sp = sp;
+			if (last_line != (unsigned int)adjust->line_no) {
+			    line_info_table[k].line_no = last_line = adjust->line_no;
+			    line_info_table[k].position = pos;
 			k++;
+			}
 			generated_iseq[pos++] = BIN(jump);
 			generated_iseq[pos++] = 0;
 		    }
@@ -1550,10 +1554,12 @@
 
     iseq->iseq = (void *)generated_iseq;
     iseq->iseq_size = pos;
-    iseq->insn_info_table = insn_info_table;
-    iseq->insn_info_size = k;
     iseq->stack_max = stack_max;
 
+    line_info_table = ruby_xrealloc(line_info_table, k * sizeof(struct iseq_line_info_entry));
+    iseq->line_info_table = line_info_table;
+    iseq->line_info_size = k;
+
     return COMPILE_OK;
 }
 
@@ -5288,6 +5294,7 @@
     long i, len = RARRAY_LEN(body);
     int j;
     int line_no = 0;
+
     /*
      * index -> LABEL *label
      */
Index: proc.c
===================================================================
--- proc.c	(revision 33045)
+++ proc.c	(revision 33046)
@@ -685,7 +685,7 @@
 
     if (!iseq) return Qnil;
     loc[0] = iseq->filename;
-    if (iseq->insn_info_table) {
+    if (iseq->line_info_table) {
 	loc[1] = INT2FIX(rb_iseq_first_lineno(iseq));
     }
     else {
@@ -823,7 +823,7 @@
     if (RUBY_VM_NORMAL_ISEQ_P(iseq)) {
 	int line_no = 0;
 
-	if (iseq->insn_info_table) {
+	if (iseq->line_info_table) {
 	    line_no = rb_iseq_first_lineno(iseq);
 	}
 	str = rb_sprintf("#<%s:%p@%s:%d%s>", cname, (void *)self,
Index: vm_method.c
===================================================================
--- vm_method.c	(revision 33045)
+++ vm_method.c	(revision 33046)
@@ -216,7 +216,7 @@
 		break;
 	    }
 	    if (iseq && !NIL_P(iseq->filename)) {
-		int line = iseq->insn_info_table ? rb_iseq_first_lineno(iseq) : 0;
+		int line = iseq->line_info_table ? rb_iseq_first_lineno(iseq) : 0;
 		rb_compile_warning(RSTRING_PTR(iseq->filename), line,
 				   "previous definition of %s was here",
 				   rb_id2name(old_def->original_id));
Index: vm.c
===================================================================
--- vm.c	(revision 33045)
+++ vm.c	(revision 33046)
@@ -716,20 +716,10 @@
     int line_no = 0;
     const rb_iseq_t *iseq = cfp->iseq;
 
-    if (RUBY_VM_NORMAL_ISEQ_P(iseq) && iseq->insn_info_size > 0) {
-	rb_num_t i;
+    if (RUBY_VM_NORMAL_ISEQ_P(iseq)) {
 	size_t pos = cfp->pc - cfp->iseq->iseq_encoded;
-
-	if (iseq->insn_info_table[0].position == pos) goto found;
-	for (i = 1; i < iseq->insn_info_size; i++) {
-	    if (iseq->insn_info_table[i].position == pos) {
-		line_no = iseq->insn_info_table[i - 1].line_no;
-		goto found;
+	line_no = rb_iseq_line_no(iseq, pos);
 	    }
-	}
-	line_no = iseq->insn_info_table[i - 1].line_no;
-    }
-  found:
     return line_no;
 }
 
Index: vm_insnhelper.c
===================================================================
--- vm_insnhelper.c	(revision 33045)
+++ vm_insnhelper.c	(revision 33046)
@@ -118,8 +118,8 @@
     if (iseq) {
 	int line_no = 1;
 
-	if (iseq->insn_info_size) {
-	    line_no = iseq->insn_info_table[0].line_no;
+	if (iseq->line_info_size) {
+	    line_no = iseq->line_info_table[0].line_no;
 	}
 
 	err_line = rb_sprintf("%s:%d:in `%s'",

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/