diff --git a/crates/gpui/src/profiler/hang.rs b/crates/gpui/src/profiler/hang.rs index 9514931225..7aea645b81 100644 --- a/crates/gpui/src/profiler/hang.rs +++ b/crates/gpui/src/profiler/hang.rs @@ -40,13 +40,30 @@ pub struct HangIncident { /// The interval the hangs occurred in, including all non-hang foreground /// work recorded alongside them. pub snapshot: FrameSnapshot, - /// The events that blocked the foreground for at least the detector's - /// threshold, longest first. When the incident was triggered by the - /// frame budget alone, no event crossed the threshold and this instead + /// Which detection rule qualified the interval. + pub trigger: HangTrigger, + /// For [`HangTrigger::Threshold`], the events that blocked the foreground + /// for at least the detector's threshold, longest first. For + /// [`HangTrigger::Budget`], no event crossed the threshold and this instead /// holds every event in the interval, longest first. pub contributors: Vec, } +/// The detection rule that qualified an interval as a [`HangIncident`]. +/// +/// Recorded explicitly so consumers can separate the two classes without +/// re-deriving them from `stall_ms`, which stops working whenever the +/// detector's thresholds change. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum HangTrigger { + /// A single event blocked the foreground for at least the hang threshold. + Threshold, + /// No single event crossed the threshold, but the interval's total + /// foreground spend reached the frame budget. + Budget, +} + impl HangDetector { /// Creates a detector reporting single events at or above `threshold` /// and intervals whose total foreground spend reached `frame_budget`. @@ -98,6 +115,8 @@ pub struct SerializedHangIncident { /// newly drawn frame finished platform submission (see /// [`HangDetector::first_present_at`]), otherwise `"steady"`. pub phase: &'static str, + /// `"threshold"` or `"budget"` (see [`HangTrigger`]). + pub trigger: HangTrigger, /// When the incident's active window started, in milliseconds since app /// startup: the sealing frame's first invalidation, or the earliest /// contributor's start when nothing was pending a repaint. Foreground @@ -250,6 +269,7 @@ impl SerializedHangIncident { Some(first_present_at) if active_start >= first_present_at => "steady", _ => "startup", }, + trigger: incident.trigger, start_ms: since_startup(active_start), active_ms: as_millis(active), stall_ms: incident @@ -407,7 +427,7 @@ impl HangIncident { .filter(|event| event.duration() >= threshold) .copied() .collect(); - if contributors.is_empty() { + let trigger = if contributors.is_empty() { if snapshot.journal_discontinuous { return None; } @@ -416,10 +436,14 @@ impl HangIncident { return None; } contributors = snapshot.events.clone(); - } + HangTrigger::Budget + } else { + HangTrigger::Threshold + }; contributors.sort_by_key(|event| std::cmp::Reverse(event.duration())); Some(Self { snapshot, + trigger, contributors, }) } @@ -449,7 +473,9 @@ mod tests { InputTiming, IntervalBoundary, PollSummary, PresentedFrame, SmallPollFlush, install_test_foreground_journal, record_present, }; - use super::{HangDetector, HangIncident, SerializedHangContributor, SerializedHangIncident}; + use super::{ + HangDetector, HangIncident, HangTrigger, SerializedHangContributor, SerializedHangIncident, + }; actions!(hang_test, [HangyAction]); @@ -567,6 +593,7 @@ mod tests { let serialized = SerializedHangIncident::convert(startup, &incident, 1, Some(at(50))); assert_eq!(serialized.phase, "steady"); + assert_eq!(serialized.trigger, HangTrigger::Threshold); // The frame's first invalidation anchors the active window, not the // interval start or the first contributor. assert_eq!(serialized.start_ms, 100.0); @@ -755,12 +782,14 @@ mod tests { let incident = HangIncident::detect(snapshot, HANG_THRESHOLD, FRAME_BUDGET) .expect("foreground spend exceeded the frame budget"); + assert_eq!(incident.trigger, HangTrigger::Budget); assert_eq!(incident.contributors.len(), 3); assert_eq!( incident.contributors[0].duration(), Duration::from_millis(8) ); let serialized = SerializedHangIncident::convert(startup, &incident, 8, Some(startup)); + assert_eq!(serialized.trigger, HangTrigger::Budget); assert_eq!(serialized.stall_ms, 8.0); assert_eq!(serialized.dirty_to_present_ms, Some(150.0)); assert_eq!(serialized.sealed_by, "present"); diff --git a/crates/zed/src/reliability/hang_detection.rs b/crates/zed/src/reliability/hang_detection.rs index bb4db32198..55423269ae 100644 --- a/crates/zed/src/reliability/hang_detection.rs +++ b/crates/zed/src/reliability/hang_detection.rs @@ -47,9 +47,9 @@ pub(crate) fn start(client: Arc, cx: &mut App) { // constantly. Duration::from_millis(100) } else { - // One 120Hz refresh: an interval that spends this much on the - // foreground has plausibly dropped a frame. - Duration::from_millis(8) + // At least one dropped frame on any display. Generous while budget + // incidents are plentiful; lower it as they get fixed. + Duration::from_millis(24) }; if cfg!(debug_assertions) { diff --git a/crates/zed/src/reliability/hang_detection/telemetry.rs b/crates/zed/src/reliability/hang_detection/telemetry.rs index 97eea191c0..c1bf9e4d5e 100644 --- a/crates/zed/src/reliability/hang_detection/telemetry.rs +++ b/crates/zed/src/reliability/hang_detection/telemetry.rs @@ -1,9 +1,9 @@ use std::time::{Duration, Instant}; -use gpui::profiler::hang::SerializedHangIncident; +use gpui::profiler::hang::{HangTrigger, SerializedHangIncident}; /// Cap on incidents per telemetry event. When more accrue between sends, the -/// ones with the largest stalls are kept and `total_incidents` still counts +/// ones with the largest stalls are kept and the incident counts still cover /// them all. const MAX_REPORTED_INCIDENTS: usize = 10; @@ -14,7 +14,8 @@ const SEND_INTERVAL: Duration = Duration::from_mins(30); pub struct Reporter { last_send: Instant, pending: Vec, - total_incidents: u64, + threshold_incidents: u64, + budget_incidents: u64, } impl Reporter { @@ -22,12 +23,16 @@ impl Reporter { Self { last_send: Instant::now(), pending: Vec::new(), - total_incidents: 0, + threshold_incidents: 0, + budget_incidents: 0, } } pub fn add(&mut self, incident: SerializedHangIncident) { - self.total_incidents += 1; + match incident.trigger { + HangTrigger::Threshold => self.threshold_incidents += 1, + HangTrigger::Budget => self.budget_incidents += 1, + } self.pending.push(incident); if self.pending.len() > MAX_REPORTED_INCIDENTS { self.pending @@ -49,8 +54,17 @@ impl Reporter { } let mut incidents = std::mem::take(&mut self.pending); incidents.sort_by(|a, b| b.stall_ms.total_cmp(&a.stall_ms)); - let total_incidents = std::mem::take(&mut self.total_incidents); + let threshold_incidents = std::mem::take(&mut self.threshold_incidents); + let budget_incidents = std::mem::take(&mut self.budget_incidents); + // `total_incidents` predates the split; existing queries key on it. + let total_incidents = threshold_incidents + budget_incidents; - telemetry::event!("Hang Incidents", incidents, total_incidents); + telemetry::event!( + "Hang Incidents", + incidents, + total_incidents, + threshold_incidents, + budget_incidents + ); } }