Fix broken stats gathering code if no. of GCs > MAX_PHASES

Author: k-sareen

src/util/statistics/counter/event_counter.rs

@@ -1,5 +1,5 @@
 use super::*;
-use crate::util::statistics::stats::{SharedStats, MAX_PHASES};
+use crate::util::statistics::stats::{SharedStats, DEFAULT_NUM_PHASES};
 use std::sync::Arc;
 
 /**
@@ -10,7 +10,7 @@ pub struct EventCounter {
     name: String,
     pub implicitly_start: bool,
     merge_phases: bool,
-    count: Box<[u64; MAX_PHASES]>,
+    count: Vec<u64>,
     current_count: u64,
     running: bool,
     stats: Arc<SharedStats>,
@@ -27,7 +27,7 @@ impl EventCounter {
             name,
             implicitly_start,
             merge_phases,
-            count: Box::new([0; MAX_PHASES]),
+            count: Vec::with_capacity(DEFAULT_NUM_PHASES),
             current_count: 0,
             running: false,
             stats,
@@ -76,13 +76,15 @@ impl Counter for EventCounter {
             return;
         }
         debug_assert!(self.running);
-        self.count[self.stats.get_phase()] = self.current_count;
+        self.count.push(self.current_count);
+        debug_assert_eq!(self.count[self.stats.get_phase()], self.current_count);
         self.running = false;
     }
 
     fn phase_change(&mut self, old_phase: usize) {
         if self.running {
-            self.count[old_phase] = self.current_count;
+            self.count.push(self.current_count);
+            debug_assert_eq!(self.count[old_phase], self.current_count);
             self.current_count = 0;
         }
     }

src/util/statistics/counter/long_counter.rs

@@ -1,13 +1,13 @@
 use super::*;
-use crate::util::statistics::stats::{SharedStats, MAX_PHASES};
+use crate::util::statistics::stats::{SharedStats, DEFAULT_NUM_PHASES};
 use std::fmt;
 use std::sync::Arc;
 
 pub struct LongCounter<T: Diffable> {
     name: String,
     pub implicitly_start: bool,
     merge_phases: bool,
-    count: Box<[u64; MAX_PHASES]>, // FIXME make this resizable
+    count: Vec<u64>,
     diffable: T,
     start_value: Option<T::Val>,
     total_count: u64,
@@ -41,15 +41,17 @@ impl<T: Diffable> Counter for LongCounter<T> {
         self.diffable.stop();
         let current_value = self.diffable.current_value();
         let delta = T::diff(&current_value, self.start_value.as_ref().unwrap());
-        self.count[self.stats.get_phase()] += delta;
+        self.count.push(delta);
+        debug_assert_eq!(self.count[self.stats.get_phase()], delta);
         self.total_count += delta;
     }
 
     fn phase_change(&mut self, old_phase: usize) {
         if self.running {
             let now = self.diffable.current_value();
             let delta = T::diff(&now, self.start_value.as_ref().unwrap());
-            self.count[old_phase] += delta;
+            self.count.push(delta);
+            debug_assert_eq!(self.count[old_phase], delta);
             self.total_count += delta;
             self.start_value = Some(now);
         }
@@ -139,7 +141,7 @@ impl<T: Diffable> LongCounter<T> {
             name,
             implicitly_start,
             merge_phases,
-            count: Box::new([0; MAX_PHASES]),
+            count: Vec::with_capacity(DEFAULT_NUM_PHASES),
             diffable,
             start_value: None,
             total_count: 0,

src/util/statistics/stats.rs

@@ -11,9 +11,8 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::Arc;
 use std::sync::Mutex;
 
-// TODO: Increasing this number would cause JikesRVM die at boot time. I don't really know why.
-// E.g. using 1 << 14 will cause JikesRVM segfault at boot time.
-pub const MAX_PHASES: usize = 1 << 12;
+/// The default number of phases for statistics.
+pub const DEFAULT_NUM_PHASES: usize = 1 << 12;
 pub const MAX_COUNTERS: usize = 100;
 
 /// GC stats shared among counters
@@ -51,10 +50,8 @@ pub struct Stats {
     // Initialization of libpfm4 is required before we can use `PerfEvent` types
     #[cfg(feature = "perf_counter")]
     perfmon: Perfmon,
-
     pub shared: Arc<SharedStats>,
     counters: Mutex<Vec<Arc<Mutex<dyn Counter + Send>>>>,
-    exceeded_phase_limit: AtomicBool,
 }
 
 impl Stats {
@@ -99,10 +96,8 @@ impl Stats {
             total_time: t,
             #[cfg(feature = "perf_counter")]
             perfmon,
-
             shared,
             counters: Mutex::new(counters),
-            exceeded_phase_limit: AtomicBool::new(false),
         }
     }
 
@@ -157,32 +152,22 @@ impl Stats {
         if !self.get_gathering_stats() {
             return;
         }
-        if self.get_phase() < MAX_PHASES - 1 {
-            let counters = self.counters.lock().unwrap();
-            for counter in &(*counters) {
-                counter.lock().unwrap().phase_change(self.get_phase());
-            }
-            self.shared.increment_phase();
-        } else if !self.exceeded_phase_limit.load(Ordering::SeqCst) {
-            eprintln!("Warning: number of GC phases exceeds MAX_PHASES");
-            self.exceeded_phase_limit.store(true, Ordering::SeqCst);
+        let counters = self.counters.lock().unwrap();
+        for counter in &(*counters) {
+            counter.lock().unwrap().phase_change(self.get_phase());
         }
+        self.shared.increment_phase();
     }
 
     pub fn end_gc(&self) {
         if !self.get_gathering_stats() {
             return;
         }
-        if self.get_phase() < MAX_PHASES - 1 {
-            let counters = self.counters.lock().unwrap();
-            for counter in &(*counters) {
-                counter.lock().unwrap().phase_change(self.get_phase());
-            }
-            self.shared.increment_phase();
-        } else if !self.exceeded_phase_limit.load(Ordering::SeqCst) {
-            eprintln!("Warning: number of GC phases exceeds MAX_PHASES");
-            self.exceeded_phase_limit.store(true, Ordering::SeqCst);
+        let counters = self.counters.lock().unwrap();
+        for counter in &(*counters) {
+            counter.lock().unwrap().phase_change(self.get_phase());
         }
+        self.shared.increment_phase();
     }
 
     pub fn print_stats<VM: VMBinding>(&self, mmtk: &'static MMTK<VM>) {