From 26ccc2eaa491531faaecc6214196f3ca082a7e06 Mon Sep 17 00:00:00 2001 From: Kelvin Nilsen Date: Fri, 17 Apr 2026 15:30:34 +0000 Subject: [PATCH] 8380651: [ubsan] gc/logging/TestGCId.java triggers runtime error: division by zero in shenandoahAdaptiveHeuristics Reviewed-by: wkemper, shade, xpeng --- .../shenandoahAdaptiveHeuristics.cpp | 34 ++++++++++++------- .../gc/shenandoah/shenandoahControlThread.cpp | 3 +- .../shenandoahGenerationalControlThread.cpp | 5 ++- .../share/gc/shenandoah/shenandoahHeap.cpp | 9 +++-- .../share/gc/shenandoah/shenandoahHeap.hpp | 2 +- .../share/gc/shenandoah/shenandoahUtils.cpp | 7 ++-- .../share/gc/shenandoah/shenandoahUtils.hpp | 3 +- 7 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp index 6b327d7e4af..c595d1fd9cd 100644 --- a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp +++ b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp @@ -209,14 +209,14 @@ void ShenandoahAdaptiveHeuristics::choose_collection_set_from_regiondata(Shenand } } -void ShenandoahAdaptiveHeuristics::add_degenerated_gc_time(double timestamp, double gc_time) { +void ShenandoahAdaptiveHeuristics::add_degenerated_gc_time(double time_at_start, double gc_time) { // Conservatively add sample into linear model If this time is above the predicted concurrent gc time - if (predict_gc_time(timestamp) < gc_time) { - add_gc_time(timestamp, gc_time); + if (predict_gc_time(time_at_start) < gc_time) { + add_gc_time(time_at_start, gc_time); } } -void ShenandoahAdaptiveHeuristics::add_gc_time(double timestamp, double gc_time) { +void ShenandoahAdaptiveHeuristics::add_gc_time(double time_at_start, double gc_time) { // Update best-fit linear predictor of GC time uint index = (_gc_time_first_sample_index + _gc_time_num_samples) % GC_TIME_SAMPLE_SIZE; if (_gc_time_num_samples == GC_TIME_SAMPLE_SIZE) { @@ -225,10 +225,10 @@ void ShenandoahAdaptiveHeuristics::add_gc_time(double timestamp, double gc_time) _gc_time_sum_of_xy -= _gc_time_xy[index]; _gc_time_sum_of_xx -= _gc_time_xx[index]; } - _gc_time_timestamps[index] = timestamp; + _gc_time_timestamps[index] = time_at_start; _gc_time_samples[index] = gc_time; - _gc_time_xy[index] = timestamp * gc_time; - _gc_time_xx[index] = timestamp * timestamp; + _gc_time_xy[index] = time_at_start * gc_time; + _gc_time_xx[index] = time_at_start * time_at_start; _gc_time_sum_of_timestamps += _gc_time_timestamps[index]; _gc_time_sum_of_samples += _gc_time_samples[index]; @@ -247,18 +247,26 @@ void ShenandoahAdaptiveHeuristics::add_gc_time(double timestamp, double gc_time) _gc_time_b = gc_time; _gc_time_sd = 0.0; } else if (_gc_time_num_samples == 2) { - // Two points define a line - double delta_y = gc_time - _gc_time_samples[_gc_time_first_sample_index]; - double delta_x = timestamp - _gc_time_timestamps[_gc_time_first_sample_index]; - _gc_time_m = delta_y / delta_x; + assert(time_at_start > _gc_time_timestamps[_gc_time_first_sample_index], + "Two GC cycles cannot finish at same time: %.6f vs %.6f, with GC times %.6f and %.6f", time_at_start, + _gc_time_timestamps[_gc_time_first_sample_index], gc_time, _gc_time_samples[_gc_time_first_sample_index]); + + // Two points define a line + double delta_x = time_at_start - _gc_time_timestamps[_gc_time_first_sample_index]; + double delta_y = gc_time - _gc_time_samples[_gc_time_first_sample_index]; + _gc_time_m = delta_y / delta_x; // y = mx + b // so b = y0 - mx0 - _gc_time_b = gc_time - _gc_time_m * timestamp; + _gc_time_b = gc_time - _gc_time_m * time_at_start; _gc_time_sd = 0.0; } else { + // Since timestamps are monotonically increasing, denominator does not equal zero. + double denominator = _gc_time_num_samples * _gc_time_sum_of_xx - _gc_time_sum_of_timestamps * _gc_time_sum_of_timestamps; + assert(denominator != 0.0, "Invariant: samples: %u, sum_of_xx: %.6f, sum_of_timestamps: %.6f", + _gc_time_num_samples, _gc_time_sum_of_xx, _gc_time_sum_of_timestamps); _gc_time_m = ((_gc_time_num_samples * _gc_time_sum_of_xy - _gc_time_sum_of_timestamps * _gc_time_sum_of_samples) / - (_gc_time_num_samples * _gc_time_sum_of_xx - _gc_time_sum_of_timestamps * _gc_time_sum_of_timestamps)); + denominator); _gc_time_b = (_gc_time_sum_of_samples - _gc_time_m * _gc_time_sum_of_timestamps) / _gc_time_num_samples; double sum_of_squared_deviations = 0.0; for (size_t i = 0; i < _gc_time_num_samples; i++) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahControlThread.cpp index 4c6e82c86a5..6175f15676c 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahControlThread.cpp @@ -346,7 +346,8 @@ void ShenandoahControlThread::service_stw_full_cycle(GCCause::Cause cause) { void ShenandoahControlThread::service_stw_degenerated_cycle(GCCause::Cause cause, ShenandoahGC::ShenandoahDegenPoint point) { assert (point != ShenandoahGC::_degenerated_unset, "Degenerated point should be set"); ShenandoahHeap* const heap = ShenandoahHeap::heap(); - ShenandoahGCSession session(cause, heap->global_generation()); + ShenandoahGCSession session(cause, heap->global_generation(), true, + point == ShenandoahGC::ShenandoahDegenPoint::_degenerated_outside_cycle); heap->increment_total_collections(false); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index aa6a4a9bab2..bbad82de1dc 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -628,11 +628,10 @@ void ShenandoahGenerationalControlThread::service_stw_full_cycle(GCCause::Cause void ShenandoahGenerationalControlThread::service_stw_degenerated_cycle(const ShenandoahGCRequest& request) { assert(_degen_point != ShenandoahGC::_degenerated_unset, "Degenerated point should be set"); - request.generation->heuristics()->record_degenerated_cycle_start(ShenandoahGC::ShenandoahDegenPoint::_degenerated_outside_cycle - == _degen_point); _heap->increment_total_collections(false); - ShenandoahGCSession session(request.cause, request.generation); + ShenandoahGCSession session(request.cause, request.generation, true, + _degen_point == ShenandoahGC::ShenandoahDegenPoint::_degenerated_outside_cycle); ShenandoahDegenGC gc(_degen_point, request.generation); gc.collect(request.cause); _degen_point = ShenandoahGC::_degenerated_unset; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp index 75d3ade4e49..4b01ea1cd52 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp @@ -1650,7 +1650,8 @@ void ShenandoahHeap::set_active_generation(ShenandoahGeneration* generation) { _active_generation = generation; } -void ShenandoahHeap::on_cycle_start(GCCause::Cause cause, ShenandoahGeneration* generation) { +void ShenandoahHeap::on_cycle_start(GCCause::Cause cause, ShenandoahGeneration* generation, + bool is_degenerated, bool is_out_of_cycle) { shenandoah_policy()->record_collection_cause(cause); const GCCause::Cause current = gc_cause(); @@ -1659,7 +1660,11 @@ void ShenandoahHeap::on_cycle_start(GCCause::Cause cause, ShenandoahGeneration* set_gc_cause(cause); - generation->heuristics()->record_cycle_start(); + if (is_degenerated) { + generation->heuristics()->record_degenerated_cycle_start(is_out_of_cycle); + } else { + generation->heuristics()->record_cycle_start(); + } } void ShenandoahHeap::on_cycle_end(ShenandoahGeneration* generation) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp index ab7dd00b774..bed26a093d0 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp @@ -564,7 +564,7 @@ public: return _evac_tracker; } - void on_cycle_start(GCCause::Cause cause, ShenandoahGeneration* generation); + void on_cycle_start(GCCause::Cause cause, ShenandoahGeneration* generation, bool is_degenerated, bool is_out_of_cycle); void on_cycle_end(ShenandoahGeneration* generation); ShenandoahVerifier* verifier(); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp b/src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp index dea47fcbf4f..5af2e274833 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp @@ -56,15 +56,14 @@ const char* ShenandoahGCSession::cycle_end_message(ShenandoahGenerationType type } } -ShenandoahGCSession::ShenandoahGCSession(GCCause::Cause cause, ShenandoahGeneration* generation) : +ShenandoahGCSession::ShenandoahGCSession(GCCause::Cause cause, ShenandoahGeneration* generation, + bool is_degenerated, bool is_out_of_cycle) : _heap(ShenandoahHeap::heap()), _generation(generation), _timer(_heap->gc_timer()), _tracer(_heap->tracer()) { assert(!ShenandoahGCPhase::is_current_phase_valid(), "No current GC phase"); - - _heap->on_cycle_start(cause, _generation); - + _heap->on_cycle_start(cause, _generation, is_degenerated, is_out_of_cycle); _timer->register_gc_start(); _tracer->report_gc_start(cause, _timer->gc_start()); _heap->trace_heap_before_gc(_tracer); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp b/src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp index 0169941c3d9..1ed6e43e3e1 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp @@ -70,7 +70,8 @@ private: static const char* cycle_end_message(ShenandoahGenerationType type); public: - ShenandoahGCSession(GCCause::Cause cause, ShenandoahGeneration* generation); + ShenandoahGCSession(GCCause::Cause cause, ShenandoahGeneration* generation, + bool is_degenerated = false, bool is_out_of_cycle = false); ~ShenandoahGCSession(); };