refactor: Remove atomics from metrics (#3968)

* replace "AtomicUsize" with "u64" for field type on "Metrics"
* move "compiler_starts" field from "Metrics" to "GlobalState"

cli/compilers/ts.rs

@@ -257,10 +257,7 @@ impl TsCompiler {
       .expect("Unable to create worker state");
 
     // Count how many times we start the compiler worker.
-    global_state
+    global_state.compiler_starts.fetch_add(1, Ordering::SeqCst);
-      .metrics
-      .compiler_starts
-      .fetch_add(1, Ordering::SeqCst);
 
     let mut worker = CompilerWorker::new(
       "TS".to_string(),

cli/compilers/wasm.rs

@@ -61,10 +61,7 @@ impl WasmCompiler {
       .expect("Unable to create worker state");
 
     // Count how many times we start the compiler worker.
-    global_state
+    global_state.compiler_starts.fetch_add(1, Ordering::SeqCst);
-      .metrics
-      .compiler_starts
-      .fetch_add(1, Ordering::SeqCst);
 
     let mut worker = CompilerWorker::new(
       "WASM".to_string(),

cli/global_state.rs

@@ -10,7 +10,6 @@ use crate::deno_error::permission_denied;
 use crate::file_fetcher::SourceFileFetcher;
 use crate::flags;
 use crate::lockfile::Lockfile;
-use crate::metrics::Metrics;
 use crate::msg;
 use crate::permissions::DenoPermissions;
 use crate::progress::Progress;
@@ -22,6 +21,7 @@ use std::env;
 use std::ops::Deref;
 use std::path::Path;
 use std::str;
+use std::sync::atomic::AtomicUsize;
 use std::sync::Arc;
 use std::sync::Mutex;
 use tokio::sync::Mutex as AsyncMutex;
@@ -39,7 +39,6 @@ pub struct GlobalStateInner {
   /// Permissions parsed from `flags`.
   pub permissions: DenoPermissions,
   pub dir: deno_dir::DenoDir,
-  pub metrics: Metrics,
   pub progress: Progress,
   pub file_fetcher: SourceFileFetcher,
   pub js_compiler: JsCompiler,
@@ -47,6 +46,7 @@ pub struct GlobalStateInner {
   pub ts_compiler: TsCompiler,
   pub wasm_compiler: WasmCompiler,
   pub lockfile: Option<Mutex<Lockfile>>,
+  pub compiler_starts: AtomicUsize,
   compile_lock: AsyncMutex<()>,
 }
 
@@ -101,7 +101,6 @@ impl GlobalState {
       dir,
       permissions: DenoPermissions::from_flags(&flags),
       flags,
-      metrics: Metrics::default(),
       progress,
       file_fetcher,
       ts_compiler,
@@ -109,6 +108,7 @@ impl GlobalState {
       json_compiler: JsonCompiler {},
       wasm_compiler: WasmCompiler::default(),
       lockfile,
+      compiler_starts: AtomicUsize::new(0),
       compile_lock: AsyncMutex::new(()),
     };
 

cli/metrics.rs

@@ -1,12 +1,10 @@
-use std::sync::atomic::AtomicUsize;
+// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-
 #[derive(Default)]
 pub struct Metrics {
-  pub ops_dispatched: AtomicUsize,
+  pub ops_dispatched: u64,
-  pub ops_completed: AtomicUsize,
+  pub ops_completed: u64,
-  pub bytes_sent_control: AtomicUsize,
+  pub bytes_sent_control: u64,
-  pub bytes_sent_data: AtomicUsize,
+  pub bytes_sent_data: u64,
-  pub bytes_received: AtomicUsize,
+  pub bytes_received: u64,
-  pub resolve_count: AtomicUsize,
+  pub resolve_count: u64,
-  pub compiler_starts: AtomicUsize,
 }

cli/ops/runtime.rs

@@ -8,7 +8,6 @@ use crate::version;
 use crate::DenoSubcommand;
 use deno_core::*;
 use std::env;
-use std::sync::atomic::Ordering;
 
 /// BUILD_OS and BUILD_ARCH match the values in Deno.build. See js/build.ts.
 #[cfg(target_os = "macos")]
@@ -59,10 +58,10 @@ fn op_metrics(
   let m = &state.metrics;
 
   Ok(JsonOp::Sync(json!({
-    "opsDispatched": m.ops_dispatched.load(Ordering::SeqCst) as u64,
+    "opsDispatched": m.ops_dispatched,
-    "opsCompleted": m.ops_completed.load(Ordering::SeqCst) as u64,
+    "opsCompleted": m.ops_completed,
-    "bytesSentControl": m.bytes_sent_control.load(Ordering::SeqCst) as u64,
+    "bytesSentControl": m.bytes_sent_control,
-    "bytesSentData": m.bytes_sent_data.load(Ordering::SeqCst) as u64,
+    "bytesSentData": m.bytes_sent_data,
-    "bytesReceived": m.bytes_received.load(Ordering::SeqCst) as u64
+    "bytesReceived": m.bytes_received
   })))
 }

cli/state.rs

@@ -74,22 +74,22 @@ impl State {
     let state = self.clone();
 
     move |control: &[u8], zero_copy: Option<ZeroCopyBuf>| -> CoreOp {
-      let bytes_sent_control = control.len();
+      let bytes_sent_control = control.len() as u64;
       let bytes_sent_zero_copy =
-        zero_copy.as_ref().map(|b| b.len()).unwrap_or(0);
+        zero_copy.as_ref().map(|b| b.len()).unwrap_or(0) as u64;
 
       let op = dispatcher(control, zero_copy);
       state.metrics_op_dispatched(bytes_sent_control, bytes_sent_zero_copy);
 
       match op {
         Op::Sync(buf) => {
-          state.metrics_op_completed(buf.len());
+          state.metrics_op_completed(buf.len() as u64);
           Op::Sync(buf)
         }
         Op::Async(fut) => {
           let state = state.clone();
           let result_fut = fut.map_ok(move |buf: Buf| {
-            state.metrics_op_completed(buf.len());
+            state.metrics_op_completed(buf.len() as u64);
             buf
           });
           Op::Async(result_fut.boxed_local())
@@ -97,7 +97,7 @@ impl State {
         Op::AsyncUnref(fut) => {
           let state = state.clone();
           let result_fut = fut.map_ok(move |buf: Buf| {
-            state.metrics_op_completed(buf.len());
+            state.metrics_op_completed(buf.len() as u64);
             buf
           });
           Op::AsyncUnref(result_fut.boxed_local())
@@ -176,9 +176,9 @@ impl Loader for State {
       }
     }
 
-    let state = self.borrow();
+    let mut state = self.borrow_mut();
     // TODO(bartlomieju): incrementing resolve_count here has no sense...
-    state.metrics.resolve_count.fetch_add(1, Ordering::SeqCst);
+    state.metrics.resolve_count += 1;
     let module_url_specified = module_specifier.to_string();
     let global_state = state.global_state.clone();
     let target_lib = state.target_lib.clone();
@@ -359,27 +359,18 @@ impl State {
 
   pub fn metrics_op_dispatched(
     &self,
-    bytes_sent_control: usize,
+    bytes_sent_control: u64,
-    bytes_sent_data: usize,
+    bytes_sent_data: u64,
   ) {
-    let state = self.borrow();
+    let mut state = self.borrow_mut();
-    state.metrics.ops_dispatched.fetch_add(1, Ordering::SeqCst);
+    state.metrics.ops_dispatched += 1;
-    state
+    state.metrics.bytes_sent_control += bytes_sent_control;
-      .metrics
+    state.metrics.bytes_sent_data += bytes_sent_data;
-      .bytes_sent_control
-      .fetch_add(bytes_sent_control, Ordering::SeqCst);
-    state
-      .metrics
-      .bytes_sent_data
-      .fetch_add(bytes_sent_data, Ordering::SeqCst);
   }
 
-  pub fn metrics_op_completed(&self, bytes_received: usize) {
+  pub fn metrics_op_completed(&self, bytes_received: u64) {
-    let state = self.borrow();
+    let mut state = self.borrow_mut();
-    state.metrics.ops_completed.fetch_add(1, Ordering::SeqCst);
+    state.metrics.ops_completed += 1;
-    state
+    state.metrics.bytes_received += bytes_received;
-      .metrics
-      .bytes_received
-      .fetch_add(bytes_received, Ordering::SeqCst);
   }
 }

cli/worker.rs

@@ -265,11 +265,10 @@ mod tests {
         panic!("Future got unexpected error: {:?}", e);
       }
     });
-    let mut state = state_.borrow_mut();
+    let state = state_.borrow();
-    let metrics = &mut state.metrics;
+    assert_eq!(state.metrics.resolve_count, 2);
-    assert_eq!(metrics.resolve_count.load(Ordering::SeqCst), 2);
     // Check that we didn't start the compiler.
-    assert_eq!(metrics.compiler_starts.load(Ordering::SeqCst), 0);
+    assert_eq!(state.global_state.compiler_starts.load(Ordering::SeqCst), 0);
   }
 
   #[test]
@@ -298,11 +297,10 @@ mod tests {
       }
     });
 
-    let mut state = state_.borrow_mut();
+    let state = state_.borrow();
-    let metrics = &mut state.metrics;
+    assert_eq!(state.metrics.resolve_count, 1);
-    // TODO  assert_eq!(metrics.resolve_count.load(Ordering::SeqCst), 2);
     // Check that we didn't start the compiler.
-    assert_eq!(metrics.compiler_starts.load(Ordering::SeqCst), 0);
+    assert_eq!(state.global_state.compiler_starts.load(Ordering::SeqCst), 0);
   }
 
   #[tokio::test]
@@ -339,13 +337,10 @@ mod tests {
     if let Err(e) = (&mut *worker).await {
       panic!("Future got unexpected error: {:?}", e);
     }
-    let state = state.borrow_mut();
+    let state = state.borrow();
-    assert_eq!(state.metrics.resolve_count.load(Ordering::SeqCst), 3);
+    assert_eq!(state.metrics.resolve_count, 3);
     // Check that we've only invoked the compiler once.
-    assert_eq!(
+    assert_eq!(state.global_state.compiler_starts.load(Ordering::SeqCst), 1);
-      global_state.metrics.compiler_starts.load(Ordering::SeqCst),
-      1
-    );
     drop(http_server_guard);
   }