From ebb28d613ef1d9af0830b6dc707957dc86017433 Mon Sep 17 00:00:00 2001 From: Ruby Date: Wed, 26 Jul 2023 13:39:31 +0900 Subject: [PATCH] `cc->cme` should not be marked. cc is callcache. cc->klass (klass) should not be marked because if the klass is free'ed, the cc->klass will be cleared by `vm_cc_invalidate()`. cc->cme (cme) should not be marked because if cc is invalidated when cme is free'ed. - klass marks cme if klass uses cme. - caller classe's ccs->cme marks cc->cme. - if cc is invalidated (klass doesn't refer the cc), cc is invalidated by `vm_cc_invalidate()` and cc->cme is not be accessed. - On the multi-Ractors, cme will be collected with global GC so that it is safe if GC is not interleaving while accessing cc and cme. fix [Bug #19436] ```ruby 10_000.times{|i| # p i if (i%1_000) == 0 str = "x" * 1_000_000 def str.foo = nil eval "def call#{i}(s) = s.foo" send "call#{i}", str } ``` Without this patch: ``` real 1m5.639s user 0m6.637s sys 0m58.292s ``` and with this patch: ``` real 0m2.045s user 0m1.627s sys 0m0.164s ``` --- gc.c | 33 +++++++++++++++++++++++---------- iseq.c | 49 ++++++++++++++++++++++++++++--------------------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/gc.c b/gc.c index ca018b7ae14114..f55fcb4fe1cc43 100644 --- a/gc.c +++ b/gc.c @@ -7015,11 +7015,22 @@ gc_mark_imemo(rb_objspace_t *objspace, VALUE obj) case imemo_callinfo: return; case imemo_callcache: - { - const struct rb_callcache *cc = (const struct rb_callcache *)obj; - // should not mark klass here - gc_mark(objspace, (VALUE)vm_cc_cme(cc)); - } + /* cc is callcache. + * + * cc->klass (klass) should not be marked because if the klass is + * free'ed, the cc->klass will be cleared by `vm_cc_invalidate()`. + * + * cc->cme (cme) should not be marked because if cc is invalidated + * when cme is free'ed. + * - klass marks cme if klass uses cme. + * - caller classe's ccs->cme marks cc->cme. + * - if cc is invalidated (klass doesn't refer the cc), + * cc is invalidated by `vm_cc_invalidate()` and cc->cme is + * not be accessed. + * - On the multi-Ractors, cme will be collected with global GC + * so that it is safe if GC is not interleaving while accessing + * cc and cme. + */ return; case imemo_constcache: { @@ -10122,12 +10133,14 @@ gc_ref_update_imemo(rb_objspace_t *objspace, VALUE obj) if (!is_live_object(objspace, cc->klass)) { *((VALUE *)(&cc->klass)) = (VALUE)0; } - } - if (cc->cme_) { - TYPED_UPDATE_IF_MOVED(objspace, struct rb_callable_method_entry_struct *, cc->cme_); - if (!is_live_object(objspace, (VALUE)cc->cme_)) { - *((struct rb_callable_method_entry_struct **)(&cc->cme_)) = (struct rb_callable_method_entry_struct *)0; + // cc->cme_ is available if cc->klass is given + + if (cc->cme_) { + TYPED_UPDATE_IF_MOVED(objspace, struct rb_callable_method_entry_struct *, cc->cme_); + if (!is_live_object(objspace, (VALUE)cc->cme_)) { + *((struct rb_callable_method_entry_struct **)(&cc->cme_)) = (struct rb_callable_method_entry_struct *)0; + } } } } diff --git a/iseq.c b/iseq.c index 540f180e2e84df..7bd255a5059fea 100644 --- a/iseq.c +++ b/iseq.c @@ -282,6 +282,29 @@ rb_iseq_mark_and_move_each_value(const rb_iseq_t *iseq, VALUE *original_iseq) } } +static bool +cc_is_active(const struct rb_callcache *cc, bool reference_updating) +{ + if (cc) { + if (reference_updating) { + cc = (const struct rb_callcache *)rb_gc_location((VALUE)cc); + } + + if (vm_cc_markable(cc)) { + if (cc->klass) { // cc is not invalidated + const struct rb_callable_method_entry_struct *cme = vm_cc_cme(cc); + if (reference_updating) { + cme = (const struct rb_callable_method_entry_struct *)rb_gc_location((VALUE)cme); + } + if (!METHOD_ENTRY_INVALIDATED(cme)) { + return true; + } + } + } + } + return false; +} + void rb_iseq_mark_and_move(rb_iseq_t *iseq, bool reference_updating) { @@ -310,27 +333,11 @@ rb_iseq_mark_and_move(rb_iseq_t *iseq, bool reference_updating) if (cds[i].ci) rb_gc_mark_and_move_ptr(&cds[i].ci); - const struct rb_callcache *cc = cds[i].cc; - if (cc) { - if (reference_updating) { - cc = (const struct rb_callcache *)rb_gc_location((VALUE)cc); - } - - if (vm_cc_markable(cc)) { - VM_ASSERT((cc->flags & VM_CALLCACHE_ON_STACK) == 0); - - const struct rb_callable_method_entry_struct *cme = vm_cc_cme(cc); - if (reference_updating) { - cme = (const struct rb_callable_method_entry_struct *)rb_gc_location((VALUE)cme); - } - - if (cc->klass && !METHOD_ENTRY_INVALIDATED(cme)) { - rb_gc_mark_and_move_ptr(&cds[i].cc); - } - else { - cds[i].cc = rb_vm_empty_cc(); - } - } + if (cc_is_active(cds[i].cc, reference_updating)) { + rb_gc_mark_and_move_ptr(&cds[i].cc); + } + else { + cds[i].cc = rb_vm_empty_cc(); } } }