From 6c618538d15b26da6f9570bffc3d4a28129305eb Mon Sep 17 00:00:00 2001 From: eileencodes Date: Wed, 24 Jul 2024 14:37:26 -0400 Subject: [PATCH 1/2] Improve nogc error When using `NoGC` if the machine runs out of memory, the user will see an error that says garbage collection failed: ``` ERROR: An MMTk GC thread panicked. This is a bug. panicked at /Users/eileencodes/src/github.com/open_source/mmtk-core/src/plan/nogc/global.rs:74:9: internal error: entered unreachable code: GC triggered in nogc Backtrace is disabled. run with `RUST_BACKTRACE=1` environment variable to display a backtrace running file: test/zlib/test_zlib.rb ``` I added a check in `poll` to see if the plan being used supports GC and if not, raise an error. I could have updated the `schedule_collection` function to return this error, but I felt that since that function is regarding scheduling, it was better to raise if any plan does not support GC than change the purpose of `schedule_collection`. We still want to know if a GC ended up being triggered in `schedule_collection`. The new error looks like this: ``` [2024-07-24T18:43:45Z INFO mmtk::memory_manager] Initialized MMTk with NoGC (FixedHeapSize(1073741824)) thread '' panicked at /Users/eileencodes/src/github.com/open_source/mmtk-core/src/util/heap/gc_trigger.rs:80:17: internal error: entered unreachable code: User ran out of space and the plan does not support collecting garbage. note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fatal runtime error: failed to initiate panic, error 5 running file: test/zlib/test_zlib.rb ``` --- src/util/heap/gc_trigger.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/util/heap/gc_trigger.rs b/src/util/heap/gc_trigger.rs index 94ba63fe6e..85de0b0561 100644 --- a/src/util/heap/gc_trigger.rs +++ b/src/util/heap/gc_trigger.rs @@ -76,17 +76,23 @@ impl GCTrigger { .policy .is_gc_required(space_full, space.map(|s| SpaceStats::new(s)), plan) { - info!( - "[POLL] {}{} ({}/{} pages)", - if let Some(space) = space { - format!("{}: ", space.get_name()) - } else { - "".to_string() - }, - "Triggering collection", - plan.get_reserved_pages(), - plan.get_total_pages(), - ); + if !plan.constraints().collects_garbage { + unreachable!("User ran out of space and the plan does not support collecting garbage."); + } + else { + info!( + "[POLL] {}{} ({}/{} pages)", + if let Some(space) = space { + format!("{}: ", space.get_name()) + } else { + "".to_string() + }, + "Triggering collection", + plan.get_reserved_pages(), + plan.get_total_pages(), + ); + } + self.gc_requester.request(); return true; } From b800ae294ccdca2634260cbdab3be965288d25a6 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 25 Jul 2024 10:01:17 +0800 Subject: [PATCH 2/2] Minor change --- src/util/heap/gc_trigger.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/util/heap/gc_trigger.rs b/src/util/heap/gc_trigger.rs index 85de0b0561..a6f2e74ebf 100644 --- a/src/util/heap/gc_trigger.rs +++ b/src/util/heap/gc_trigger.rs @@ -76,21 +76,20 @@ impl GCTrigger { .policy .is_gc_required(space_full, space.map(|s| SpaceStats::new(s)), plan) { + info!( + "[POLL] {}{} ({}/{} pages)", + if let Some(space) = space { + format!("{}: ", space.get_name()) + } else { + "".to_string() + }, + "Triggering collection", + plan.get_reserved_pages(), + plan.get_total_pages(), + ); + if !plan.constraints().collects_garbage { - unreachable!("User ran out of space and the plan does not support collecting garbage."); - } - else { - info!( - "[POLL] {}{} ({}/{} pages)", - if let Some(space) = space { - format!("{}: ", space.get_name()) - } else { - "".to_string() - }, - "Triggering collection", - plan.get_reserved_pages(), - plan.get_total_pages(), - ); + panic!("User ran out of space and the plan does not support collecting garbage."); } self.gc_requester.request();