Skip to content
Browse files

* 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().



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@33046 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
  • Loading branch information...
1 parent 4f03f0c commit 7049d9c80d1ba859beb5d68c7a9de37d5c11b8e8 @ko1 ko1 committed Aug 24, 2011
Showing with 110 additions and 84 deletions.
  1. +24 −0 ChangeLog
  2. +24 −17 compile.c
  3. +49 −44 iseq.c
  4. +4 −4 iseq.h
  5. +2 −2 proc.c
  6. +2 −12 vm.c
  7. +2 −2 vm_core.h
  8. +2 −2 vm_insnhelper.c
  9. +1 −1 vm_method.c
View
24 ChangeLog
@@ -1,3 +1,27 @@
+Wed Aug 24 15:13:56 2011 Koichi Sasada <ko1@atdot.net>
+
+ * 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@atdot.net>
* insns.def (defined): fix to checking class variable.
View
41 compile.c
@@ -51,7 +51,7 @@ typedef struct iseq_label_data {
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 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
{
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 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
/* 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 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
"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 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
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;
- pos += len;
+ 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;
break;
}
case ISEQ_ELEMENT_LABEL:
@@ -1512,19 +1514,21 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
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_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
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 @@ iseq_build_from_ary_body(rb_iseq_t *iseq, LINK_ANCHOR *anchor,
long i, len = RARRAY_LEN(body);
int j;
int line_no = 0;
+
/*
* index -> LABEL *label
*/
View
93 iseq.c
@@ -78,7 +78,7 @@ iseq_free(void *ptr)
}
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 @@ iseq_memsize(const void *ptr)
}
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 @@ rb_iseq_first_lineno(rb_iseq_t *iseq)
/* 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;
+
+ 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);
- for (i = 0; i < size; i++) {
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 @@ find_line_no(rb_iseq_t *iseq, unsigned long pos)
}
}
-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 @@ rb_iseq_disasm_insn(VALUE ret, VALUE *iseq, size_t pos,
}
}
- 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 @@ cdhash_each(VALUE key, VALUE value, VALUE ary)
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 @@ iseq_data_to_ary(rb_iseq_t *iseq)
/* 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 @@ iseq_data_to_ary(rb_iseq_t *iseq)
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 @@ VALUE
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 @@ rb_iseq_build_for_ruby2cext(
} \
} 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);
View
8 iseq.h
@@ -26,6 +26,7 @@ VALUE rb_iseq_build_from_ary(rb_iseq_t *iseq, VALUE locals, VALUE args,
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 @@ struct rb_compile_option_struct {
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 {
View
4 proc.c
@@ -685,7 +685,7 @@ iseq_location(rb_iseq_t *iseq)
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 @@ proc_to_s(VALUE self)
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,
View
14 vm.c
@@ -716,20 +716,10 @@ rb_vm_get_sourceline(const rb_control_frame_t *cfp)
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;
}
View
4 vm_core.h
@@ -176,8 +176,8 @@ struct rb_iseq_struct {
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;
View
4 vm_insnhelper.c
@@ -118,8 +118,8 @@ argument_error(const rb_iseq_t *iseq, int miss_argc, int correct_argc)
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'",
View
2 vm_method.c
@@ -216,7 +216,7 @@ rb_method_entry_make(VALUE klass, ID mid, rb_method_type_t type,
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));

0 comments on commit 7049d9c

Please sign in to comment.
Something went wrong with that request. Please try again.