From 920c17dc94239baae05b513046b27967f11a3569 Mon Sep 17 00:00:00 2001 From: KJ Tsanaktsidis Date: Thu, 1 Feb 2024 15:33:30 +1100 Subject: [PATCH] Backport #20157 to Ruby 3.3 (#9428) * Fix GC.measure_total_time regression Commit 93ac7405b80cc61930d73da04441fa09af1851e1 introduced a regression where measurements would still be taken after setting GC.measure_total_time = false. Fixes [Bug #20157] * Add test case for GC.measure_total_time --------- Co-authored-by: Rian McGuire --- gc.c | 24 ++++++++++++++++-------- test/ruby/test_gc.rb | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/gc.c b/gc.c index 6d62ca293d..6419f8ff25 100644 --- a/gc.c +++ b/gc.c @@ -9749,10 +9749,6 @@ gc_enter_count(enum gc_enter_event event) } } -#ifndef MEASURE_GC -#define MEASURE_GC (objspace->flags.measure_gc) -#endif - static bool current_process_time(struct timespec *ts); static void @@ -9822,12 +9818,18 @@ gc_exit(rb_objspace_t *objspace, enum gc_enter_event event, unsigned int *lock_l RB_VM_LOCK_LEAVE_LEV(lock_lev); } +#ifndef MEASURE_GC +#define MEASURE_GC (objspace->flags.measure_gc) +#endif + static void gc_marking_enter(rb_objspace_t *objspace) { GC_ASSERT(during_gc != 0); - gc_clock_start(&objspace->profile.marking_start_time); + if (MEASURE_GC) { + gc_clock_start(&objspace->profile.marking_start_time); + } } static void @@ -9835,7 +9837,9 @@ gc_marking_exit(rb_objspace_t *objspace) { GC_ASSERT(during_gc != 0); - objspace->profile.marking_time_ns += gc_clock_end(&objspace->profile.marking_start_time); + if (MEASURE_GC) { + objspace->profile.marking_time_ns += gc_clock_end(&objspace->profile.marking_start_time); + } } static void @@ -9843,7 +9847,9 @@ gc_sweeping_enter(rb_objspace_t *objspace) { GC_ASSERT(during_gc != 0); - gc_clock_start(&objspace->profile.sweeping_start_time); + if (MEASURE_GC) { + gc_clock_start(&objspace->profile.sweeping_start_time); + } } static void @@ -9851,7 +9857,9 @@ gc_sweeping_exit(rb_objspace_t *objspace) { GC_ASSERT(during_gc != 0); - objspace->profile.sweeping_time_ns += gc_clock_end(&objspace->profile.sweeping_start_time); + if (MEASURE_GC) { + objspace->profile.sweeping_time_ns += gc_clock_end(&objspace->profile.sweeping_start_time); + } } static void * diff --git a/test/ruby/test_gc.rb b/test/ruby/test_gc.rb index 4c4a7f6a27..39b001c3d0 100644 --- a/test/ruby/test_gc.rb +++ b/test/ruby/test_gc.rb @@ -231,6 +231,23 @@ class TestGc < Test::Unit::TestCase assert_equal stat[:total_freed_objects], stat_heap_sum[:total_freed_objects] end + def test_measure_total_time + assert_separately([], __FILE__, __LINE__, <<~RUBY) + GC.measure_total_time = false + + time_before = GC.stat(:time) + + # Generate some garbage + Random.new.bytes(100 * 1024 * 1024) + GC.start + + time_after = GC.stat(:time) + + # If time measurement is disabled, the time stat should not change + assert_equal time_before, time_after + RUBY + end + def test_latest_gc_info omit 'stress' if GC.stress