ruby-changes:69834
From: Yusuke <ko1@a...>
Date: Sun, 21 Nov 2021 08:59:38 +0900 (JST)
Subject: [ruby-changes:69834] feda058531 (master): Refactor hacky ID tables to struct rb_ast_id_table_t
https://git.ruby-lang.org/ruby.git/commit/?id=feda058531 From feda058531c0bdd5b673180accb4407dcc798c79 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh <mame@r...> Date: Thu, 18 Nov 2021 03:40:49 +0900 Subject: Refactor hacky ID tables to struct rb_ast_id_table_t The implementation of a local variable tables was represented as `ID*`, but it was very hacky: the first element is not an ID but the size of the table, and, the last element is (sometimes) a link to the next local table only when the id tables are a linked list. This change converts the hacky implementation to a normal struct. --- ast.c | 6 +++--- common.mk | 2 ++ compile.c | 28 ++++++++++++---------------- node.c | 47 +++++++++++++++++++++++++++++++++++------------ node.h | 12 ++++++++++-- parse.y | 29 +++++++++++++---------------- vm.c | 9 ++++----- 7 files changed, 79 insertions(+), 54 deletions(-) diff --git a/ast.c b/ast.c index 96116d43eba..8139fcd390d 100644 --- a/ast.c +++ b/ast.c @@ -600,11 +600,11 @@ node_children(rb_ast_t *ast, const NODE *node) https://github.com/ruby/ruby/blob/trunk/ast.c#L600 } case NODE_SCOPE: { - ID *tbl = node->nd_tbl; - int i, size = tbl ? (int)*tbl++ : 0; + rb_ast_id_table_t *tbl = node->nd_tbl; + int i, size = tbl ? tbl->size : 0; VALUE locals = rb_ary_new_capa(size); for (i = 0; i < size; i++) { - rb_ary_push(locals, var_name(tbl[i])); + rb_ary_push(locals, var_name(tbl->ids[i])); } return rb_ary_new_from_args(3, locals, NEW_CHILD(ast, node->nd_args), NEW_CHILD(ast, node->nd_body)); } diff --git a/common.mk b/common.mk index 7ba32ac6e8f..6b7378eeb61 100644 --- a/common.mk +++ b/common.mk @@ -7937,6 +7937,7 @@ localeinit.$(OBJEXT): {$(VPATH)}st.h https://github.com/ruby/ruby/blob/trunk/common.mk#L7937 localeinit.$(OBJEXT): {$(VPATH)}subst.h main.$(OBJEXT): $(hdrdir)/ruby.h main.$(OBJEXT): $(hdrdir)/ruby/ruby.h +main.$(OBJEXT): $(top_srcdir)/internal/compilers.h main.$(OBJEXT): {$(VPATH)}assert.h main.$(OBJEXT): {$(VPATH)}backward.h main.$(OBJEXT): {$(VPATH)}backward/2/assume.h @@ -8463,6 +8464,7 @@ math.$(OBJEXT): {$(VPATH)}missing.h https://github.com/ruby/ruby/blob/trunk/common.mk#L8464 math.$(OBJEXT): {$(VPATH)}st.h math.$(OBJEXT): {$(VPATH)}subst.h memory_view.$(OBJEXT): $(hdrdir)/ruby/ruby.h +memory_view.$(OBJEXT): $(top_srcdir)/internal/compilers.h memory_view.$(OBJEXT): $(top_srcdir)/internal/hash.h memory_view.$(OBJEXT): $(top_srcdir)/internal/variable.h memory_view.$(OBJEXT): {$(VPATH)}assert.h diff --git a/compile.c b/compile.c index a6505f82d6d..d3cb079562f 100644 --- a/compile.c +++ b/compile.c @@ -482,7 +482,7 @@ static int iseq_setup_insn(rb_iseq_t *iseq, LINK_ANCHOR *const anchor); https://github.com/ruby/ruby/blob/trunk/compile.c#L482 static int iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor); static int iseq_insns_unification(rb_iseq_t *iseq, LINK_ANCHOR *const anchor); -static int iseq_set_local_table(rb_iseq_t *iseq, const ID *tbl); +static int iseq_set_local_table(rb_iseq_t *iseq, const rb_ast_id_table_t *tbl); static int iseq_set_exception_local_table(rb_iseq_t *iseq); static int iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const anchor, const NODE *const node); @@ -1946,21 +1946,13 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons https://github.com/ruby/ruby/blob/trunk/compile.c#L1946 } static int -iseq_set_local_table(rb_iseq_t *iseq, const ID *tbl) +iseq_set_local_table(rb_iseq_t *iseq, const rb_ast_id_table_t *tbl) { - unsigned int size; - - if (tbl) { - size = (unsigned int)*tbl; - tbl++; - } - else { - size = 0; - } + unsigned int size = tbl ? tbl->size : 0; if (size > 0) { ID *ids = (ID *)ALLOC_N(ID, size); - MEMCPY(ids, tbl, ID, size); + MEMCPY(ids, tbl->ids, ID, size); iseq->body->local_table = ids; } iseq->body->local_table_size = size; @@ -7908,17 +7900,20 @@ compile_builtin_mandatory_only_method(rb_iseq_t *iseq, const NODE *node, const N https://github.com/ruby/ruby/blob/trunk/compile.c#L7900 // local table without non-mandatory parameters const int skip_local_size = iseq->body->param.size - iseq->body->param.lead_num; const int table_size = iseq->body->local_table_size - skip_local_size; - ID *tbl = ALLOCA_N(ID, table_size + 1); - tbl[0] = table_size; + + VALUE idtmp = 0; + rb_ast_id_table_t *tbl = ALLOCV(idtmp, sizeof(rb_ast_id_table_t) + table_size * sizeof(ID)); + tbl->size = table_size; + int i; // lead parameters for (i=0; i<iseq->body->param.lead_num; i++) { - tbl[i+1] = iseq->body->local_table[i]; + tbl->ids[i] = iseq->body->local_table[i]; } // local variables for (; i<table_size; i++) { - tbl[i+1] = iseq->body->local_table[i + skip_local_size]; + tbl->ids[i] = iseq->body->local_table[i + skip_local_size]; } NODE scope_node; @@ -7939,6 +7934,7 @@ compile_builtin_mandatory_only_method(rb_iseq_t *iseq, const NODE *node, const N https://github.com/ruby/ruby/blob/trunk/compile.c#L7934 ISEQ_TYPE_METHOD, ISEQ_COMPILE_DATA(iseq)->option); GET_VM()->builtin_inline_index = prev_inline_index; + ALLOCV_END(idtmp); return COMPILE_OK; } diff --git a/node.c b/node.c index 3dea7fe398e..180142d0893 100644 --- a/node.c +++ b/node.c @@ -1043,12 +1043,12 @@ dump_node(VALUE buf, VALUE indent, int comment, const NODE * node) https://github.com/ruby/ruby/blob/trunk/node.c#L1043 ANN("new scope"); ANN("format: [nd_tbl]: local table, [nd_args]: arguments, [nd_body]: body"); F_CUSTOM1(nd_tbl, "local table") { - ID *tbl = node->nd_tbl; + rb_ast_id_table_t *tbl = node->nd_tbl; int i; - int size = tbl ? (int)*tbl++ : 0; + int size = tbl ? tbl->size : 0; if (size == 0) A("(empty)"); for (i = 0; i < size; i++) { - A_ID(tbl[i]); if (i < size - 1) A(","); + A_ID(tbl->ids[i]); if (i < size - 1) A(","); } } F_NODE(nd_args, "arguments"); @@ -1162,7 +1162,7 @@ typedef struct { https://github.com/ruby/ruby/blob/trunk/node.c#L1162 struct node_buffer_struct { node_buffer_list_t unmarkable; node_buffer_list_t markable; - ID *local_tables; + struct rb_ast_local_table_link *local_tables; VALUE mark_hash; }; @@ -1205,15 +1205,22 @@ node_buffer_list_free(node_buffer_list_t * nb) https://github.com/ruby/ruby/blob/trunk/node.c#L1205 } } +struct rb_ast_local_table_link { + struct rb_ast_local_table_link *next; + // struct rb_ast_id_table { + int size; + ID ids[FLEX_ARY_LEN]; + // } +}; + static void rb_node_buffer_free(node_buffer_t *nb) { node_buffer_list_free(&nb->unmarkable); node_buffer_list_free(&nb->markable); - ID * local_table = nb->local_tables; + struct rb_ast_local_table_link *local_table = nb->local_tables; while (local_table) { - unsigned int size = (unsigned int)*local_table; - ID * next_table = (ID *)local_table[size + 1]; + struct rb_ast_local_table_link *next_table = local_table->next; xfree(local_table); local_table = next_table; } @@ -1277,12 +1284,28 @@ rb_ast_node_type_change(NODE *n, enum node_type type) https://github.com/ruby/ruby/blob/trunk/node.c#L1284 } } -void -rb_ast_add_local_table(rb_ast_t *ast, ID *buf) +rb_ast_id_table_t * +rb_ast_new_local_table(rb_ast_t *ast, int size) +{ + size_t alloc_size = sizeof(struct rb_ast_local_table_link) + size * sizeof(ID); + struct rb_ast_local_table_link *link = ruby_xmalloc(alloc_size); + link->next = ast->node_buffer->local_tables; + ast->node_buffer->local_tables = link; + link->size = size; + + return (rb_ast_id_table_t *) &link->size; +} + +rb_ast_id_table_t * +rb_ast_resize_latest_local_table(rb_ast_t *ast, int size) { - unsigned int size = (unsigned int)*buf; - buf[size + 1] = (ID)ast->node_buffer->local_tables; - ast->node_buffer->local_tables = buf; + struct rb_ast_local_table_link *link = ast->node_buffer->local_tables; + size_t alloc_size = sizeof(struct rb_ast_local_table_link) + size * sizeof(ID); + link = ruby_xrealloc(link, alloc_size); + ast->node_buffer->local_tables = link; + link->size = size; + + return (rb_ast_id_table_t *) &link->size; } void diff --git a/node.h b/node.h index 84504556242..bd1c9d1fbf1 100644 --- a/node.h +++ b/node.h @@ -11,6 +11,8 @@ https://github.com/ruby/ruby/blob/trunk/node.h#L11 **********************************************************************/ +#include "internal/compilers.h" + #if defined(__cplusplus) extern "C" { #if 0 @@ -146,13 +148,18 @@ code_loc_gen(const rb_code_location_t *loc1, const rb_code_location_t *loc2) https://github.com/ruby/ruby/blob/trunk/node.h#L148 return loc; } +typedef struct rb_ast_id_table { + int size; + ID ids[FLEX_ARY_LEN]; +} rb_ast_id_table_t; + typedef struct RNode { VALUE flags; union { struct RNode *node; ID id; VALUE value; - ID *tbl; + rb_ast_id_table_t *tbl; } u1; union { struct RNode *node; @@ -411,13 +418,14 @@ typedef struct rb_ast_struct { https://github.com/ruby/ruby/blob/trunk/node.h#L418 rb_ast_t *rb_ast_new(void); void rb_ast_mark(rb_ast_t*); void rb_ast_update_references(rb_ast_t*); -void rb_ast_add_local_table(rb_ast_t*, ID *buf); void rb_ast_dispose(rb_ast_t*); void rb_ast_free(rb_ast_t*); size_t rb_ast_memsize(const rb_ast_t*); void rb_ast_add_mark_object(rb_ast_t*, VALUE); NODE *rb_ast_newnode(rb_ast_t*, enum node_type type); void rb_ast_delete_node(rb_ast_t*, NODE *n); +rb_ast_id_table_t *rb_ast_new_local_table(rb_ast_t*, int); +rb_ast_id_table_t *rb_ast_resize_latest_local_table(rb_ast_t*, int); VALUE rb_parser_new(void); VALUE rb_parser_end_seen_p(VALUE); diff --git a/parse.y b/parse.y index b6498651bfb..d386561f6ac 100644 --- a/parse.y +++ b/parse.y @@ -574,7 +574,7 @@ static (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/