refactor(lsp): cleanup partially locking methods (#23723)

1 files changed, 94 insertions, 54 deletions

diff --git a/cli/lsp/performance.rs b/cli/lsp/performance.rs
index 6a89c237e..b3dc53b28 100644
--- a/cli/lsp/performance.rs
+++ b/cli/lsp/performance.rs
@@ -8,6 +8,7 @@ use std::cmp;
 use std::collections::HashMap;
 use std::collections::VecDeque;
 use std::fmt;
+use std::sync::Arc;
 use std::time::Duration;
 use std::time::Instant;
 
@@ -70,22 +71,56 @@ impl From<PerformanceMark> for PerformanceMeasure {
   }
 }
 
-/// A simple structure for marking a start of something to measure the duration
-/// of and measuring that duration.  Each measurement is identified by a string
-/// name and a counter is incremented each time a new measurement is marked.
-///
-/// The structure will limit the size of measurements to the most recent 1000,
-/// and will roll off when that limit is reached.
 #[derive(Debug)]
-pub struct Performance {
-  counts: Mutex<HashMap<String, u32>>,
-  measurements_by_type:
-    Mutex<HashMap<String, (/* count */ u32, /* duration */ f64)>>,
+pub struct PerformanceScopeMark {
+  performance_inner: Arc<Mutex<PerformanceInner>>,
+  inner: Option<PerformanceMark>,
+}
+
+impl Drop for PerformanceScopeMark {
+  fn drop(&mut self) {
+    self
+      .performance_inner
+      .lock()
+      .measure(self.inner.take().unwrap());
+  }
+}
+
+#[derive(Debug)]
+struct PerformanceInner {
+  counts: HashMap<String, u32>,
+  measurements_by_type: HashMap<String, (/* count */ u32, /* duration */ f64)>,
   max_size: usize,
-  measures: Mutex<VecDeque<PerformanceMeasure>>,
+  measures: VecDeque<PerformanceMeasure>,
 }
 
-impl Default for Performance {
+impl PerformanceInner {
+  fn measure(&mut self, mark: PerformanceMark) -> Duration {
+    let measure = PerformanceMeasure::from(mark);
+    lsp_debug!(
+      "{},",
+      json!({
+        "type": "measure",
+        "name": measure.name,
+        "count": measure.count,
+        "duration": measure.duration.as_micros() as f64 / 1000.0,
+      })
+    );
+    let duration = measure.duration;
+    let measurement = self
+      .measurements_by_type
+      .entry(measure.name.to_string())
+      .or_insert((0, 0.0));
+    measurement.1 += duration.as_micros() as f64 / 1000.0;
+    self.measures.push_front(measure);
+    while self.measures.len() > self.max_size {
+      self.measures.pop_back();
+    }
+    duration
+  }
+}
+
+impl Default for PerformanceInner {
   fn default() -> Self {
     Self {
       counts: Default::default(),
@@ -96,12 +131,21 @@ impl Default for Performance {
   }
 }
 
+/// A simple structure for marking a start of something to measure the duration
+/// of and measuring that duration.  Each measurement is identified by a string
+/// name and a counter is incremented each time a new measurement is marked.
+///
+/// The structure will limit the size of measurements to the most recent 1000,
+/// and will roll off when that limit is reached.
+#[derive(Debug, Default)]
+pub struct Performance(Arc<Mutex<PerformanceInner>>);
+
 impl Performance {
   /// Return the count and average duration of a measurement identified by name.
   #[cfg(test)]
   pub fn average(&self, name: &str) -> Option<(usize, Duration)> {
     let mut items = Vec::new();
-    for measure in self.measures.lock().iter() {
+    for measure in self.0.lock().measures.iter() {
       if measure.name == name {
         items.push(measure.duration);
       }
@@ -120,7 +164,7 @@ impl Performance {
   /// of each measurement.
   pub fn averages(&self) -> Vec<PerformanceAverage> {
     let mut averages: HashMap<String, Vec<Duration>> = HashMap::new();
-    for measure in self.measures.lock().iter() {
+    for measure in self.0.lock().measures.iter() {
       averages
         .entry(measure.name.clone())
         .or_default()
@@ -141,8 +185,10 @@ impl Performance {
   }
 
   pub fn measurements_by_type(&self) -> Vec<(String, u32, f64)> {
-    let measurements_by_type = self.measurements_by_type.lock();
-    measurements_by_type
+    self
+      .0
+      .lock()
+      .measurements_by_type
       .iter()
       .map(|(name, (count, duration))| (name.to_string(), *count, *duration))
       .collect::<Vec<_>>()
@@ -150,7 +196,7 @@ impl Performance {
 
   pub fn averages_as_f64(&self) -> Vec<(String, u32, f64)> {
     let mut averages: HashMap<String, Vec<Duration>> = HashMap::new();
-    for measure in self.measures.lock().iter() {
+    for measure in self.0.lock().measures.iter() {
       averages
         .entry(measure.name.clone())
         .or_default()
@@ -171,17 +217,18 @@ impl Performance {
     name: S,
     maybe_args: Option<V>,
   ) -> PerformanceMark {
+    let mut inner = self.0.lock();
     let name = name.as_ref();
-    let mut counts = self.counts.lock();
-    let count = counts.entry(name.to_string()).or_insert(0);
-    *count += 1;
-    {
-      let mut measurements_by_type = self.measurements_by_type.lock();
-      let measurement = measurements_by_type
-        .entry(name.to_string())
-        .or_insert((0, 0.0));
-      measurement.0 += 1;
-    }
+    let count = *inner
+      .counts
+      .entry(name.to_string())
+      .and_modify(|c| *c += 1)
+      .or_insert(1);
+    inner
+      .measurements_by_type
+      .entry(name.to_string())
+      .and_modify(|(c, _)| *c += 1)
+      .or_insert((1, 0.0));
     let msg = if let Some(args) = maybe_args {
       json!({
         "type": "mark",
@@ -198,7 +245,7 @@ impl Performance {
     lsp_debug!("{},", msg);
     PerformanceMark {
       name: name.to_string(),
-      count: *count,
+      count,
       start: Instant::now(),
     }
   }
@@ -221,39 +268,32 @@ impl Performance {
     self.mark_inner(name, Some(args))
   }
 
+  /// Creates a performance mark which will be measured against on drop. Use
+  /// like this:
+  /// ```rust
+  /// let _mark = self.performance.measure_scope("foo");
+  /// ```
+  /// Don't use like this:
+  /// ```rust
+  /// // ❌
+  /// let _ = self.performance.measure_scope("foo");
+  /// ```
+  pub fn measure_scope<S: AsRef<str>>(&self, name: S) -> PerformanceScopeMark {
+    PerformanceScopeMark {
+      performance_inner: self.0.clone(),
+      inner: Some(self.mark(name)),
+    }
+  }
+
   /// A function which accepts a previously created performance mark which will
   /// be used to finalize the duration of the span being measured, and add the
   /// measurement to the internal buffer.
   pub fn measure(&self, mark: PerformanceMark) -> Duration {
-    let measure = PerformanceMeasure::from(mark);
-    lsp_debug!(
-      "{},",
-      json!({
-        "type": "measure",
-        "name": measure.name,
-        "count": measure.count,
-        "duration": measure.duration.as_micros() as f64 / 1000.0,
-      })
-    );
-    let duration = measure.duration;
-    {
-      let mut measurements_by_type = self.measurements_by_type.lock();
-      let measurement = measurements_by_type
-        .entry(measure.name.to_string())
-        .or_insert((0, 0.0));
-      measurement.1 += duration.as_micros() as f64 / 1000.0;
-    }
-    let mut measures = self.measures.lock();
-    measures.push_front(measure);
-    while measures.len() > self.max_size {
-      measures.pop_back();
-    }
-    duration
+    self.0.lock().measure(mark)
   }
 
   pub fn to_vec(&self) -> Vec<PerformanceMeasure> {
-    let measures = self.measures.lock();
-    measures.iter().cloned().collect()
+    self.0.lock().measures.iter().cloned().collect()
   }
 }