From 7049d9c80d1ba859beb5d68c7a9de37d5c11b8e8 Mon Sep 17 00:00:00 2001 From: ko1 Date: Wed, 24 Aug 2011 06:31:15 +0000 Subject: * 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 --- ChangeLog | 24 +++++++++++++++ compile.c | 41 ++++++++++++++----------- iseq.c | 93 ++++++++++++++++++++++++++++++--------------------------- iseq.h | 8 ++--- proc.c | 4 +-- vm.c | 14 ++------- vm_core.h | 4 +-- vm_insnhelper.c | 4 +-- vm_method.c | 2 +- 9 files changed, 110 insertions(+), 84 deletions(-) diff --git a/ChangeLog b/ChangeLog index 54f3d52267..4d48fb8428 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +Wed Aug 24 15:13:56 2011 Koichi Sasada + + * 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 * insns.def (defined): fix to checking class variable. diff --git a/compile.c b/compile.c index 53149cafb2..fd800a8840 100644 --- a/compile.c +++ b/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 */ diff --git a/iseq.c b/iseq.c index 7b83b9e6db..bd7be02c4f 100644 --- a/iseq.c +++ b/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 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; iinsn_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); diff --git a/iseq.h b/iseq.h index beeacbb236..39f139ff94 100644 --- a/iseq.h +++ b/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 { diff --git a/proc.c b/proc.c index 0e9f34e852..299a220a60 100644 --- a/proc.c +++ b/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, diff --git a/vm.c b/vm.c index 16d04db312..bacb5ed7d3 100644 --- a/vm.c +++ b/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; } diff --git a/vm_core.h b/vm_core.h index 66c3cda792..0120def4b9 100644 --- a/vm_core.h +++ b/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; diff --git a/vm_insnhelper.c b/vm_insnhelper.c index a35dbb1196..918c7fb7de 100644 --- a/vm_insnhelper.c +++ b/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'", diff --git a/vm_method.c b/vm_method.c index 3ef93314c8..608760f2ad 100644 --- a/vm_method.c +++ b/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)); -- cgit v1.2.3