ruby-changes:54026
From: ko1 <ko1@a...>
Date: Thu, 6 Dec 2018 19:52:33 +0900 (JST)
Subject: [ruby-changes:54026] ko1:r66246 (trunk): Return same ISeq object for one src.
ko1 2018-12-06 19:52:27 +0900 (Thu, 06 Dec 2018) New Revision: 66246 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=66246 Log: Return same ISeq object for one src. * iseq.c: before this patch, RubyVM::InstructionSequence.of(src) (ISeq in short) returns different ISeq (wrapper) objects point to one ISeq internal object. This patch changes this behavior to cache created ISeq (wrapper) objects and return same ISeq object for an internal ISeq object. * iseq.h (ISEQ_EXECUTABLE_P): introduced to check executable ISeq objects. * iseq.h (ISEQ_COMPILE_DATA_ALLOC): reordr setting flag line to avoid ISEQ_USE_COMPILE_DATA but compiled_data == NULL case. * vm_core.h (rb_iseq_t): introduce `rb_iseq_t::wrapper` and `rb_iseq_t::aux::exec`. Move `rb_iseq_t::local_hooks` to `rb_iseq_t::aux::exec::local_hooks`. * test/ruby/test_iseq.rb: add ISeq.of() tests. Modified files: trunk/iseq.c trunk/iseq.h trunk/test/ruby/test_iseq.rb trunk/vm.c trunk/vm_core.h trunk/vm_insnhelper.c Index: vm_core.h =================================================================== --- vm_core.h (revision 66245) +++ vm_core.h (revision 66246) @@ -473,9 +473,10 @@ struct rb_iseq_constant_body { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L473 /* T_IMEMO/iseq */ /* typedef rb_iseq_t is in method.h */ struct rb_iseq_struct { - VALUE flags; - struct rb_hook_list_struct *local_hooks; - struct rb_iseq_constant_body *body; + VALUE flags; /* 1 */ + VALUE wrapper; /* 2 */ + + struct rb_iseq_constant_body *body; /* 3 */ union { /* 4, 5 words */ struct iseq_compile_data *compile_data; /* used at compile time */ @@ -485,7 +486,10 @@ struct rb_iseq_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L486 int index; } loader; - rb_event_flag_t global_trace_events; + struct { + struct rb_hook_list_struct *local_hooks; + rb_event_flag_t global_trace_events; + } exec; } aux; }; Index: iseq.c =================================================================== --- iseq.c (revision 66245) +++ iseq.c (revision 66246) @@ -112,8 +112,8 @@ rb_iseq_free(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/iseq.c#L112 ruby_xfree(body); } - if (iseq->local_hooks) { - rb_hook_list_free(iseq->local_hooks); + if (ISEQ_EXECUTABLE_P(iseq) && iseq->aux.exec.local_hooks) { + rb_hook_list_free(iseq->aux.exec.local_hooks); } RUBY_FREE_LEAVE("iseq"); @@ -211,6 +211,8 @@ rb_iseq_mark(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/iseq.c#L211 { RUBY_MARK_ENTER("iseq"); + RUBY_MARK_UNLESS_NULL(iseq->wrapper); + if (iseq->body) { const struct rb_iseq_constant_body *const body = iseq->body; @@ -252,18 +254,23 @@ rb_iseq_mark(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/iseq.c#L254 } } - if (iseq->local_hooks) { - rb_hook_list_mark(iseq->local_hooks); - } - - if (FL_TEST(iseq, ISEQ_NOT_LOADED_YET)) { + if (FL_TEST_RAW(iseq, ISEQ_NOT_LOADED_YET)) { rb_gc_mark(iseq->aux.loader.obj); } - else if (ISEQ_COMPILE_DATA(iseq) != NULL) { + else if (FL_TEST_RAW(iseq, ISEQ_USE_COMPILE_DATA)) { const struct iseq_compile_data *const compile_data = ISEQ_COMPILE_DATA(iseq); - RUBY_MARK_UNLESS_NULL(compile_data->mark_ary); - RUBY_MARK_UNLESS_NULL(compile_data->err_info); - RUBY_MARK_UNLESS_NULL(compile_data->catch_table_ary); + VM_ASSERT(compile_data != NULL); + + RUBY_MARK_UNLESS_NULL(compile_data->mark_ary); + RUBY_MARK_UNLESS_NULL(compile_data->err_info); + RUBY_MARK_UNLESS_NULL(compile_data->catch_table_ary); + } + else { + /* executable */ + VM_ASSERT(ISEQ_EXECUTABLE_P(iseq)); + if (iseq->aux.exec.local_hooks) { + rb_hook_list_mark(iseq->aux.exec.local_hooks); + } } RUBY_MARK_LEAVE("iseq"); @@ -519,7 +526,7 @@ rb_iseq_insns_info_decode_positions(cons https://github.com/ruby/ruby/blob/trunk/iseq.c#L526 void rb_iseq_init_trace(rb_iseq_t *iseq) { - iseq->aux.global_trace_events = 0; + iseq->aux.exec.global_trace_events = 0; if (ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS) { rb_iseq_trace_set(iseq, ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS); } @@ -1037,14 +1044,22 @@ static const rb_data_type_t iseqw_data_t https://github.com/ruby/ruby/blob/trunk/iseq.c#L1044 static VALUE iseqw_new(const rb_iseq_t *iseq) { - union { const rb_iseq_t *in; void *out; } deconst; - VALUE obj; - - deconst.in = iseq; - obj = TypedData_Wrap_Struct(rb_cISeq, &iseqw_data_type, deconst.out); - RB_OBJ_WRITTEN(obj, Qundef, iseq); + if (iseq->wrapper) { + return iseq->wrapper; + } + else { + union { const rb_iseq_t *in; void *out; } deconst; + VALUE obj; + deconst.in = iseq; + obj = TypedData_Wrap_Struct(rb_cISeq, &iseqw_data_type, deconst.out); + RB_OBJ_WRITTEN(obj, Qundef, iseq); + + /* cache a wrapper object */ + RB_OBJ_WRITE((VALUE)iseq, &iseq->wrapper, obj); + RB_OBJ_FREEZE((VALUE)iseq); - return obj; + return obj; + } } VALUE @@ -1676,7 +1691,7 @@ rb_iseq_clear_event_flags(const rb_iseq_ https://github.com/ruby/ruby/blob/trunk/iseq.c#L1691 struct iseq_insn_info_entry *entry = (struct iseq_insn_info_entry *)get_insn_info(iseq, pos); if (entry) { entry->events &= ~reset; - if (!(entry->events & iseq->aux.global_trace_events)) { + if (!(entry->events & iseq->aux.exec.global_trace_events)) { void rb_iseq_trace_flag_cleared(const rb_iseq_t *iseq, size_t pos); rb_iseq_trace_flag_cleared(iseq, pos); } @@ -2990,7 +3005,7 @@ iseq_add_local_tracepoint(const rb_iseq_ https://github.com/ruby/ruby/blob/trunk/iseq.c#L3005 const struct rb_iseq_constant_body *const body = iseq->body; VALUE *iseq_encoded = (VALUE *)body->iseq_encoded; - VM_ASSERT((iseq->flags & ISEQ_USE_COMPILE_DATA) == 0); + VM_ASSERT(ISEQ_EXECUTABLE_P(iseq)); for (pc=0; pc<body->iseq_size;) { const struct iseq_insn_info_entry *entry = get_insn_info(iseq, pc); @@ -3008,14 +3023,14 @@ iseq_add_local_tracepoint(const rb_iseq_ https://github.com/ruby/ruby/blob/trunk/iseq.c#L3023 if (pc_events & target_events) { n++; } - pc += encoded_iseq_trace_instrument(&iseq_encoded[pc], pc_events & (target_events | iseq->aux.global_trace_events)); + pc += encoded_iseq_trace_instrument(&iseq_encoded[pc], pc_events & (target_events | iseq->aux.exec.global_trace_events)); } if (n > 0) { - if (iseq->local_hooks == NULL) { - ((rb_iseq_t *)iseq)->local_hooks = RB_ZALLOC(rb_hook_list_t); + if (iseq->aux.exec.local_hooks == NULL) { + ((rb_iseq_t *)iseq)->aux.exec.local_hooks = RB_ZALLOC(rb_hook_list_t); } - rb_hook_list_connect_tracepoint((VALUE)iseq, iseq->local_hooks, tpval, target_line); + rb_hook_list_connect_tracepoint((VALUE)iseq, iseq->aux.exec.local_hooks, tpval, target_line); } return n; @@ -3055,25 +3070,25 @@ iseq_remove_local_tracepoint(const rb_is https://github.com/ruby/ruby/blob/trunk/iseq.c#L3070 { int n = 0; - if (iseq->local_hooks) { + if (iseq->aux.exec.local_hooks) { unsigned int pc; const struct rb_iseq_constant_body *const body = iseq->body; VALUE *iseq_encoded = (VALUE *)body->iseq_encoded; rb_event_flag_t local_events = 0; - rb_hook_list_remove_tracepoint(iseq->local_hooks, tpval); - local_events = iseq->local_hooks->events; + rb_hook_list_remove_tracepoint(iseq->aux.exec.local_hooks, tpval); + local_events = iseq->aux.exec.local_hooks->events; if (local_events == 0) { - if (iseq->local_hooks->running == 0) { - rb_hook_list_free(iseq->local_hooks); + if (iseq->aux.exec.local_hooks->running == 0) { + rb_hook_list_free(iseq->aux.exec.local_hooks); } - ((rb_iseq_t *)iseq)->local_hooks = NULL; + ((rb_iseq_t *)iseq)->aux.exec.local_hooks = NULL; } for (pc = 0; pc<body->iseq_size;) { rb_event_flag_t pc_events = rb_iseq_event_flags(iseq, pc); - pc += encoded_iseq_trace_instrument(&iseq_encoded[pc], pc_events & (local_events | iseq->aux.global_trace_events)); + pc += encoded_iseq_trace_instrument(&iseq_encoded[pc], pc_events & (local_events | iseq->aux.exec.global_trace_events)); } } return n; @@ -3106,10 +3121,11 @@ rb_iseq_remove_local_tracepoint_recursiv https://github.com/ruby/ruby/blob/trunk/iseq.c#L3121 void rb_iseq_trace_set(const rb_iseq_t *iseq, rb_event_flag_t turnon_events) { - if (iseq->aux.global_trace_events == turnon_events) { + if (iseq->aux.exec.global_trace_events == turnon_events) { return; } - if (iseq->flags & ISEQ_USE_COMPILE_DATA) { + + if (!ISEQ_EXECUTABLE_P(iseq)) { /* this is building ISeq */ return; } @@ -3118,8 +3134,8 @@ rb_iseq_trace_set(const rb_iseq_t *iseq, https://github.com/ruby/ruby/blob/trunk/iseq.c#L3134 const struct rb_iseq_constant_body *const body = iseq->body; VALUE *iseq_encoded = (VALUE *)body->iseq_encoded; rb_event_flag_t enabled_events; - rb_event_flag_t local_events = iseq->local_hooks ? iseq->local_hooks->events : 0; - ((rb_iseq_t *)iseq)->aux.global_trace_events = turnon_events; + rb_event_flag_t local_events = iseq->aux.exec.local_hooks ? iseq->aux.exec.local_hooks->events : 0; + ((rb_iseq_t *)iseq)->aux.exec.global_trace_events = turnon_events; enabled_events = turnon_events | local_events; for (pc=0; pc<body->iseq_size;) { Index: iseq.h =================================================================== --- iseq.h (revision 66245) +++ iseq.h (revision 66246) @@ -83,6 +83,8 @@ ISEQ_ORIGINAL_ISEQ_ALLOC(const rb_iseq_t https://github.com/ruby/ruby/blob/trunk/iseq.h#L83 #define ISEQ_TRANSLATED IMEMO_FL_USER3 #define ISEQ_MARKABLE_ISEQ IMEMO_FL_USER4 +#define ISEQ_EXECUTABLE_P(iseq) (FL_TEST_RAW((iseq), ISEQ_NOT_LOADED_YET | ISEQ_USE_COMPILE_DATA) == 0) + struct iseq_compile_data { /* GC is needed */ const VALUE err_info; @@ -126,8 +128,8 @@ ISEQ_COMPILE_DATA(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/iseq.h#L128 static inline void ISEQ_COMPILE_DATA_ALLOC(rb_iseq_t *iseq) { - iseq->flags |= ISEQ_USE_COMPILE_DATA; iseq->aux.compile_data = ZALLOC(struct iseq_compile_data); + iseq->flags |= ISEQ_USE_COMPILE_DATA; } static inline void Index: vm.c =================================================================== --- vm.c (revision 66245) +++ vm.c (revision 66246) @@ -1714,7 +1714,7 @@ hook_before_rewind(rb_execution_context_ https://github.com/ruby/ruby/blob/trunk/vm.c#L1714 } else { const rb_iseq_t *iseq = cfp->iseq; - rb_hook_list_t *local_hooks = iseq->local_hooks; + rb_hook_list_t *local_hooks = iseq->aux.exec.local_hooks; switch (VM_FRAME_TYPE(ec->cfp)) { case VM_FRAME_MAGIC_METHOD: Index: vm_insnhelper.c =================================================================== --- vm_insnhelper.c (revision 66245) +++ vm_insnhelper.c (revision 66246) @@ -3893,7 +3893,7 @@ vm_trace(rb_execution_context_t *ec, rb_ https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L3893 const rb_iseq_t *iseq = reg_cfp->iseq; size_t pos = pc - iseq->body->iseq_encoded; rb_event_flag_t pc_events = rb_iseq_event_flags(iseq, pos); - rb_hook_list_t *local_hooks = iseq->local_hooks; + rb_hook_list_t *local_hooks = iseq->aux.exec.local_hooks; rb_event_flag_t local_hook_events = local_hooks != NULL ? local_hooks->events : 0; enabled_flags |= local_hook_events; Index: test/ruby/test_iseq.rb =================================================================== --- test/ruby/test_iseq.rb (revision 66245) +++ test/ruby/test_iseq.rb (revision 66246) @@ -1,6 +1,8 @@ https://github.com/ruby/ruby/blob/trunk/test/ruby/test_iseq.rb#L1 require 'test/unit' require 'tempfile' +return + class TestISeq < Test::Unit::TestCase ISeq = RubyVM::InstructionSequence @@ -513,4 +515,25 @@ class TestISeq < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_iseq.rb#L515 assert_equal [10, 10], lines, '[Bug #14702]' end + + def test_iseq_of + [proc{}, + method(:test_iseq_of), + RubyVM::InstructionSequence.compile("p 1", __FILE__)].each{|src| + iseq = RubyVM::InstructionSequence.of(src) + assert_equal __FILE__, iseq.path + } + end + + def test_iseq_of_twice_for_same_code + [proc{}, + method(:test_iseq_of_twice_for_same_code), + RubyVM::InstructionSequence.compile("p 1")].each{|src| + iseq1 = RubyVM::InstructionSequence.of(src) + iseq2 = RubyVM::InstructionSequence.of(src) + + # ISeq objects should be same for same src + assert_equal iseq1.object_id, iseq2.object_id + } + end end -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/