From c8c7d1ab23c98b6e00fbf273457c67383f5f4d16 Mon Sep 17 00:00:00 2001 From: dearblue Date: Tue, 23 Apr 2024 22:25:54 +0900 Subject: [PATCH 1/2] Don't `mrb_realloc_simple()` call `mrb_full_gc()` in the sweep phase `mrb_env_unshare()` calls `mrb_realloc_simple()` and follows `mrb_full_gc()` to avoid an infinite loop where `mrb_env_unshare()` is called again. This does not occur at this time, but may occur in subsequent patches. --- src/gc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gc.c b/src/gc.c index f3c7e3c6b..2833824e9 100644 --- a/src/gc.c +++ b/src/gc.c @@ -190,10 +190,12 @@ mrb_realloc_simple(mrb_state *mrb, void *p, size_t len) void *p2; #if defined(MRB_GC_STRESS) && defined(MRB_DEBUG) - mrb_full_gc(mrb); + if (mrb->gc.state != MRB_GC_STATE_SWEEP) { + mrb_full_gc(mrb); + } #endif p2 = (mrb->allocf)(mrb, p, len, mrb->allocf_ud); - if (!p2 && len > 0 && mrb->gc.heaps) { + if (!p2 && len > 0 && mrb->gc.heaps && mrb->gc.state != MRB_GC_STATE_SWEEP) { mrb_full_gc(mrb); p2 = (mrb->allocf)(mrb, p, len, mrb->allocf_ud); } From f1c9260ada6fd566c95a07516f99937bdb7cefb2 Mon Sep 17 00:00:00 2001 From: dearblue Date: Tue, 23 Apr 2024 22:30:32 +0900 Subject: [PATCH 2/2] Allow recycling fibers by GC if not referenced directly The patch assumes that `struct REnv::cxt` only performs checks with the `OP_BREAK` and `OP_RETURN_BLK` instructions, and does not reference the entity. Therefore, by changing to a weak reference, it is possible to collect fibers that are no longer directly referenced while in the suspended state. However, we need to detach the living env objects that remain in the call stack of the fiber. So, in effect, it involves a revert of following commits. - commit a3365d8b3fc957b1d1fd89526440358aa9402fc3 - commit 57ffa1c1508f01434806b1b7301f4ae982eb9ae0 Examples of the effects of change are shown below. Note that it was built with `rake MRUBY_CONFIG=host-debug`. ```ruby f = Fiber.new { (x, y, z) = "X", "Y", "Z"; Fiber.yield -> { [x, y, z] } } g = f.resume GC.start p ObjectSpace.memsize_of_all # => 59532 g.call # => ["X", "Y", "Z"] f = nil GC.start ObjectSpace.memsize_of_all # BEFORE => 59532 # AFTER => 58044 g.call # => ["X", "Y", "Z"] ``` --- src/gc.c | 20 ++++++++++++++++---- src/vm.c | 1 - 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/gc.c b/src/gc.c index 2833824e9..8c3b8d5b8 100644 --- a/src/gc.c +++ b/src/gc.c @@ -630,9 +630,8 @@ gc_mark_children(mrb_state *mrb, mrb_gc *gc, struct RBasic *obj) { struct REnv *e = (struct REnv*)obj; - if (MRB_ENV_ONSTACK_P(e) && e->cxt && e->cxt->fib) { - mrb_gc_mark(mrb, (struct RBasic*)e->cxt->fib); - } + // The data stack must always be protected from GC regardless of the MRB_ENV_CLOSE flag. + // This is because the data stack is not protected if the fiber is GC'd. mrb_int len = MRB_ENV_LEN(e); for (mrb_int i=0; istack[i]); @@ -773,7 +772,20 @@ obj_free(mrb_state *mrb, struct RBasic *obj, mrb_bool end) { struct mrb_context *c = ((struct RFiber*)obj)->cxt; - if (c != mrb->root_c) { + if (c && c != mrb->root_c) { + if (!end && c->status != MRB_FIBER_TERMINATED) { + mrb_callinfo *ci = c->ci; + mrb_callinfo *ce = c->cibase; + + while (ce <= ci) { + struct REnv *e = ci->u.env; + if (e && !is_dead(&mrb->gc, (struct RBasic*)e) && + e->tt == MRB_TT_ENV && MRB_ENV_ONSTACK_P(e)) { + mrb_env_unshare(mrb, e, TRUE); + } + ci--; + } + } mrb_free_context(mrb, c); } } diff --git a/src/vm.c b/src/vm.c index c9b007951..6e321b2f8 100644 --- a/src/vm.c +++ b/src/vm.c @@ -429,7 +429,6 @@ mrb_env_unshare(mrb_state *mrb, struct REnv *e, mrb_bool noraise) { if (e == NULL) return TRUE; if (!MRB_ENV_ONSTACK_P(e)) return TRUE; - if (e->cxt != mrb->c) return TRUE; e->cxt = NULL; /* make possible to GC the fiber that generated the env */