ruby-changes:70410
From: Koichi <ko1@a...>
Date: Tue, 21 Dec 2021 15:21:43 +0900 (JST)
Subject: [ruby-changes:70410] ad450c9fe5 (master): make `overloaded_cme_table` truly weak key map
https://git.ruby-lang.org/ruby.git/commit/?id=ad450c9fe5 From ad450c9fe51c51626dd07f6e766f47d839cae3ba Mon Sep 17 00:00:00 2001 From: Koichi Sasada <ko1@a...> Date: Tue, 21 Dec 2021 14:06:02 +0900 Subject: make `overloaded_cme_table` truly weak key map `overloaded_cme_table` keeps cme -> monly_cme pairs to manage corresponding `monly_cme` for `cme`. The lifetime of the `monly_cme` should be longer than `monly_cme`, but the previous patch losts the reference to the living `monly_cme`. Now `overloaded_cme_table` values are always root (keys are only weak reference), it means `monly_cme` does not freed until corresponding `cme` is invalidated. To make managing easy, move `overloaded_cme_table` to `rb_vm_t`. --- gc.c | 8 +------- vm.c | 6 +++++- vm_core.h | 1 + vm_method.c | 38 +++++++++++++++----------------------- 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/gc.c b/gc.c index 5fd46defb4d..c4cac9bd076 100644 --- a/gc.c +++ b/gc.c @@ -6379,9 +6379,7 @@ mark_method_entry(rb_objspace_t *objspace, const rb_method_entry_t *me) https://github.com/ruby/ruby/blob/trunk/gc.c#L6379 if (def->body.iseq.iseqptr) gc_mark(objspace, (VALUE)def->body.iseq.iseqptr); gc_mark(objspace, (VALUE)def->body.iseq.cref); if (def->iseq_overload && me->defined_class) { // cme - const rb_callable_method_entry_t *monly_cme = rb_vm_lookup_overloaded_cme((const rb_callable_method_entry_t *)me); - if (monly_cme) { - gc_mark(objspace, (VALUE)monly_cme); + if (rb_vm_lookup_overloaded_cme((const rb_callable_method_entry_t *)me)) { gc_mark_and_pin(objspace, (VALUE)me); } } @@ -10113,9 +10111,6 @@ gc_ref_update(void *vstart, void *vend, size_t stride, rb_objspace_t * objspace, https://github.com/ruby/ruby/blob/trunk/gc.c#L10111 extern rb_symbols_t ruby_global_symbols; #define global_symbols ruby_global_symbols - -st_table *rb_vm_overloaded_cme_table(void); - static void gc_update_references(rb_objspace_t *objspace) { @@ -10151,7 +10146,6 @@ gc_update_references(rb_objspace_t *objspace) https://github.com/ruby/ruby/blob/trunk/gc.c#L10146 gc_update_table_refs(objspace, objspace->id_to_obj_tbl); gc_update_table_refs(objspace, global_symbols.str_sym); gc_update_table_refs(objspace, finalizer_table); - gc_update_table_refs(objspace, rb_vm_overloaded_cme_table()); } static VALUE diff --git a/vm.c b/vm.c index 0d423cd979b..287fe2547d9 100644 --- a/vm.c +++ b/vm.c @@ -2540,6 +2540,8 @@ rb_vm_update_references(void *ptr) https://github.com/ruby/ruby/blob/trunk/vm.c#L2540 vm->top_self = rb_gc_location(vm->top_self); vm->orig_progname = rb_gc_location(vm->orig_progname); + rb_gc_update_tbl_refs(vm->overloaded_cme_table); + if (vm->coverages) { vm->coverages = rb_gc_location(vm->coverages); vm->me2counter = rb_gc_location(vm->me2counter); @@ -2637,9 +2639,10 @@ rb_vm_mark(void *ptr) https://github.com/ruby/ruby/blob/trunk/vm.c#L2639 rb_mark_tbl(vm->loading_table); } - rb_gc_mark_values(RUBY_NSIG, vm->trap_list.cmd); + rb_gc_mark_values(RUBY_NSIG, vm->trap_list.cmd); rb_id_table_foreach_values(vm->negative_cme_table, vm_mark_negative_cme, NULL); + rb_mark_tbl_no_pin(vm->overloaded_cme_table); for (i=0; i<VM_GLOBAL_CC_CACHE_TABLE_SIZE; i++) { const struct rb_callcache *cc = vm->global_cc_cache_table[i]; @@ -3801,6 +3804,7 @@ Init_BareVM(void) https://github.com/ruby/ruby/blob/trunk/vm.c#L3804 vm->objspace = rb_objspace_alloc(); ruby_current_vm_ptr = vm; vm->negative_cme_table = rb_id_table_create(16); + vm->overloaded_cme_table = st_init_numtable(); Init_native_thread(th); th->vm = vm; diff --git a/vm_core.h b/vm_core.h index 6bbce23078f..11866b85e5d 100644 --- a/vm_core.h +++ b/vm_core.h @@ -714,6 +714,7 @@ typedef struct rb_vm_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L714 int builtin_inline_index; struct rb_id_table *negative_cme_table; + st_table *overloaded_cme_table; // cme -> overloaded_cme #ifndef VM_GLOBAL_CC_CACHE_TABLE_SIZE #define VM_GLOBAL_CC_CACHE_TABLE_SIZE 1023 diff --git a/vm_method.c b/vm_method.c index f71145576a8..b8197e14cad 100644 --- a/vm_method.c +++ b/vm_method.c @@ -150,7 +150,7 @@ static rb_method_entry_t *rb_method_entry_alloc(ID called_id, VALUE owner, VALUE https://github.com/ruby/ruby/blob/trunk/vm_method.c#L150 const rb_method_entry_t * rb_method_entry_clone(const rb_method_entry_t *src_me); static const rb_callable_method_entry_t *complemented_callable_method_entry(VALUE klass, ID id); static const rb_callable_method_entry_t *lookup_overloaded_cme(const rb_callable_method_entry_t *cme); -static void delete_overloaded_cme(const rb_callable_method_entry_t *cme); + static void clear_method_cache_by_id_in_class(VALUE klass, ID mid) @@ -216,7 +216,6 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L216 rb_callable_method_entry_t *monly_cme = (rb_callable_method_entry_t *)lookup_overloaded_cme(cme); if (monly_cme) { vm_cme_invalidate(monly_cme); - delete_overloaded_cme(monly_cme); } } } @@ -393,6 +392,8 @@ rb_method_definition_release(rb_method_definition_t *def, int complemented) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L392 } } +static void delete_overloaded_cme(const rb_callable_method_entry_t *cme); + void rb_free_method_entry(const rb_method_entry_t *me) { @@ -922,12 +923,12 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil https://github.com/ruby/ruby/blob/trunk/vm_method.c#L923 } static rb_method_entry_t *rb_method_entry_alloc(ID called_id, VALUE owner, VALUE defined_class, const rb_method_definition_t *def); -static st_table *overloaded_cme_table; -st_table * -rb_vm_overloaded_cme_table(void) +static st_table * +overloaded_cme_table(void) { - return overloaded_cme_table; + VM_ASSERT(GET_VM()->overloaded_cme_table != NULL); + return GET_VM()->overloaded_cme_table; } #if VM_CHECK_MODE > 0 @@ -943,7 +944,7 @@ void https://github.com/ruby/ruby/blob/trunk/vm_method.c#L944 rb_vm_dump_overloaded_cme_table(void) { fprintf(stderr, "== rb_vm_dump_overloaded_cme_table\n"); - st_foreach(overloaded_cme_table, vm_dump_overloaded_cme_table, 0); + st_foreach(overloaded_cme_table(), vm_dump_overloaded_cme_table, 0); } #endif @@ -956,10 +957,7 @@ lookup_overloaded_cme_i(st_data_t *key, st_data_t *value, st_data_t data, int ex https://github.com/ruby/ruby/blob/trunk/vm_method.c#L957 const rb_callable_method_entry_t **ptr = (const rb_callable_method_entry_t **)data; if (rb_objspace_garbage_object_p((VALUE)cme) || - rb_objspace_garbage_object_p((VALUE)monly_cme) || - METHOD_ENTRY_INVALIDATED(cme) || - METHOD_ENTRY_INVALIDATED(monly_cme)) { - + rb_objspace_garbage_object_p((VALUE)monly_cme)) { *ptr = NULL; return ST_DELETE; } @@ -977,14 +975,8 @@ lookup_overloaded_cme(const rb_callable_method_entry_t *cme) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L975 ASSERT_vm_locking(); const rb_callable_method_entry_t *monly_cme = NULL; - st_update(overloaded_cme_table, (st_data_t)cme, lookup_overloaded_cme_i, (st_data_t)&monly_cme); - - if (monly_cme) { - return monly_cme; - } - else { - return NULL; - } + st_update(overloaded_cme_table(), (st_data_t)cme, lookup_overloaded_cme_i, (st_data_t)&monly_cme); + return monly_cme; } // used by gc.c @@ -998,7 +990,7 @@ static void https://github.com/ruby/ruby/blob/trunk/vm_method.c#L990 delete_overloaded_cme(const rb_callable_method_entry_t *cme) { ASSERT_vm_locking(); - st_delete(overloaded_cme_table, (st_data_t *)&cme, NULL); + st_delete(overloaded_cme_table(), (st_data_t *)&cme, NULL); } static const rb_callable_method_entry_t * @@ -1006,7 +998,7 @@ get_overloaded_cme(const rb_callable_method_entry_t *cme) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L998 { const rb_callable_method_entry_t *monly_cme = lookup_overloaded_cme(cme); - if (monly_cme) { + if (monly_cme && !METHOD_ENTRY_INVALIDATED(monly_cme)) { return monly_cme; } else { @@ -1021,7 +1013,7 @@ get_overloaded_cme(const rb_callable_method_entry_t *cme) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L1013 def); ASSERT_vm_locking(); - st_insert(overloaded_cme_table, (st_data_t)cme, (st_data_t)me); + st_insert(overloaded_cme_table(), (st_data_t)cme, (st_data_t)me); METHOD_ENTRY_VISI_SET(me, METHOD_ENTRY_VISI(cme)); return (rb_callable_method_entry_t *)me; @@ -2828,7 +2820,7 @@ obj_respond_to_missing(VALUE obj, VALUE mid, VALUE priv) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L2820 void Init_Method(void) { - overloaded_cme_table = st_init_numtable(); + // } void -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/