Implement a first solution for collecting metrics during a Nickel eva…

…luation (#1633)

* Implement prototype metrics collection and reporting

* Add a few sample metrics

* Document some of the rationale

* Remove unused `metrics-utils`

* Don't enable metrics by default

* Report metrics to stderr

Cargo.toml

@@ -90,6 +90,8 @@ typed-arena = "2.0.2"
 unicode-segmentation = "1.10.1"
 void = "1"
 
+metrics = "0.21"
+
 topiary = { git = "https://github.com/tweag/topiary.git", rev = "8299a04bf83c4a2774cbbff7a036c022efa939b3" }
 topiary-queries = { git = "https://github.com/tweag/topiary.git", rev = "8299a04bf83c4a2774cbbff7a036c022efa939b3", package = "topiary-queries", default-features = false, features = ["nickel"] }
 # This should be kept in sync with the revision in topiary

cli/Cargo.toml

@@ -19,7 +19,8 @@ bench = false
 default = ["repl", "doc", "format"]
 repl = ["nickel-lang-core/repl"]
 doc = ["nickel-lang-core/doc"]
-format = ["nickel-lang-core/format", "tempfile"]
+format = ["nickel-lang-core/format", "dep:tempfile"]
+metrics = ["dep:metrics", "nickel-lang-core/metrics"]
 
 [dependencies]
 nickel-lang-core = { workspace = true, features = [ "markdown" ], default-features = false }
@@ -33,6 +34,8 @@ tempfile = { workspace = true, optional = true }
 git-version = { workspace = true }
 clap_complete = { workspace = true }
 
+metrics = { workspace = true, optional = true }
+
 [dev-dependencies]
 nickel-lang-utils.workspace = true
 test-generator.workspace = true

cli/src/cli.rs

@@ -46,6 +46,11 @@ pub struct GlobalOptions {
  /// Configure when to output messages in color
  #[arg(long, global = true, value_enum, default_value_t)]
  pub color: clap::ColorChoice,
+
+ #[cfg(feature = "metrics")]
+ /// Print all recorded metrics at the very end of the program
+ #[arg(long, global = true, default_value_t = false)]
+ pub metrics: bool,
 }
 
 /// Available subcommands.

cli/src/main.rs

@@ -4,6 +4,8 @@
 mod doc;
 #[cfg(feature = "format")]
 mod format;
+#[cfg(feature = "metrics")]
+mod metrics;
 #[cfg(feature = "repl")]
 mod repl;
 
@@ -23,8 +25,14 @@ use std::process::ExitCode;
 use crate::cli::{Command, Options};
 
 fn main() -> ExitCode {
+ #[cfg(feature = "metrics")]
+ let metrics = metrics::Recorder::install();
+
  let opts = <Options as clap::Parser>::parse();
 
+ #[cfg(feature = "metrics")]
+ let report_metrics = opts.global.metrics;
+
  let result = match opts.command {
  Command::Eval(eval) => eval.run(opts.global),
  Command::PprintAst(pprint_ast) => pprint_ast.run(opts.global),
@@ -43,6 +51,11 @@ fn main() -> ExitCode {
  Command::Format(format) => format.run(opts.global),
  };
 
+ #[cfg(feature = "metrics")]
+ if report_metrics {
+ metrics.report();
+ }
+
  if let Err(e) = result {
  e.report();
  ExitCode::FAILURE

cli/src/metrics.rs

@@ -0,0 +1,131 @@
+use std::{
+ collections::HashMap,
+ hash::BuildHasherDefault,
+ sync::{atomic::Ordering, Arc, PoisonError, RwLock},
+};
+
+use metrics::{
+ atomics::AtomicU64, Counter, Gauge, Histogram, Key, KeyHasher, KeyName, SharedString, Unit,
+};
+
+/// The actual store for recorded metrics and their descriptions. We only need
+/// to take the write lock once when creating a new metric, in all other cases
+/// we only take a read lock. The `metrics` crate implements the necessary
+/// trait [`::metrics::CounterFn`] for `Arc<AtomicU64>` and in any case expects its
+/// counter types to be `Arc`s. For this reason, we use `Arc<AtomicU64>` as the
+/// value type of the counters `HashMap`.
+///
+/// The `metrics` crate expects to be used in a thread safe manner, so follow
+/// suit here.
+#[derive(Default)]
+pub(super) struct Registry {
+ counters: RwLock<HashMap<Key, Arc<AtomicU64>, BuildHasherDefault<KeyHasher>>>,
+ descriptions: RwLock<HashMap<KeyName, SharedString>>,
+}
+
+/// A metrics recorder utilizing [`Registry`] for storing metrics and their
+/// descriptions. It currently only supports [`Counter`]s and will panic if
+/// gauges or histograms are submitted.
+pub(super) struct Recorder {
+ inner: Arc<Registry>,
+}
+
+impl Recorder {
+ /// Construct a `Recorder` and register it as the global metrics recorder.
+ /// The return value is a reference to the metrics registry. This way we can
+ /// retrieve the stored values later.
+ ///
+ /// This function should only be called once at the start of the CLI.
+ pub(super) fn install() -> Arc<Registry> {
+ let registry = Arc::<Registry>::default();
+ metrics::set_boxed_recorder(Box::new(Recorder {
+ inner: registry.clone(),
+ }))
+ .expect("registering a metrics recorder shouldn't fail");
+ registry
+ }
+}
+
+impl metrics::Recorder for Recorder {
+ fn describe_counter(&self, key: KeyName, _unit: Option<Unit>, description: SharedString) {
+ self.inner
+ .descriptions
+ .write()
+ .unwrap_or_else(PoisonError::into_inner)
+ .entry(key)
+ .or_insert(description);
+ }
+
+ fn describe_gauge(&self, _key: KeyName, _unit: Option<Unit>, _description: SharedString) {
+ unimplemented!()
+ }
+
+ fn describe_histogram(&self, _key: KeyName, _unit: Option<Unit>, _description: SharedString) {
+ unimplemented!()
+ }
+
+ fn register_counter(&self, key: &Key) -> Counter {
+ let read = self
+ .inner
+ .counters
+ .read()
+ .unwrap_or_else(PoisonError::into_inner);
+ if let Some(counter) = read.get(key) {
+ Counter::from_arc(counter.clone())
+ } else {
+ drop(read);
+ let mut write = self
+ .inner
+ .counters
+ .write()
+ .unwrap_or_else(PoisonError::into_inner);
+ if let Some(counter) = write.get(key) {
+ Counter::from_arc(counter.clone())
+ } else {
+ let counter = Arc::new(AtomicU64::new(0));
+ write.insert(key.clone(), counter.clone());
+ Counter::from_arc(counter)
+ }
+ }
+ }
+
+ fn register_gauge(&self, _key: &Key) -> Gauge {
+ unimplemented!()
+ }
+
+ fn register_histogram(&self, _key: &Key) -> Histogram {
+ unimplemented!()
+ }
+}
+
+impl Registry {
+ /// Print out a report of all metrics that have been collected so far, with
+ /// their description if available.
+ ///
+ /// We expect this function to be called once at the end of the CLI, but it
+ /// should be safe to call it in the middle of a Nickel execution as well.
+ pub(super) fn report(&self) {
+ for (key, counter) in self
+ .counters
+ .read()
+ .unwrap_or_else(PoisonError::into_inner)
+ .iter()
+ {
+ if let Some(description) = self
+ .descriptions
+ .read()
+ .unwrap_or_else(PoisonError::into_inner)
+ .get(key.name())
+ {
+ eprintln!(
+ "{}: {}\n {}",
+ key.name(),
+ description,
+ counter.load(Ordering::Relaxed)
+ );
+ } else {
+ eprintln!("{}: {}", key.name(), counter.load(Ordering::Relaxed));
+ }
+ }
+ }
+}

core/Cargo.toml

@@ -15,11 +15,12 @@ bench = false
 
 [features]
 default = ["markdown", "repl", "doc", "format"]
-markdown = ["termimad"]
-repl = ["rustyline", "rustyline-derive", "ansi_term"]
-repl-wasm = ["wasm-bindgen", "js-sys", "serde_repr"]
-doc = ["comrak"]
-format = ["topiary", "topiary-queries", "tree-sitter-nickel"]
+markdown = ["dep:termimad"]
+repl = ["dep:rustyline", "dep:rustyline-derive", "dep:ansi_term"]
+repl-wasm = ["dep:wasm-bindgen", "dep:js-sys", "dep:serde_repr"]
+doc = ["dep:comrak"]
+format = ["dep:topiary", "dep:topiary-queries", "dep:tree-sitter-nickel"]
+metrics = ["dep:metrics"]
 
 [build-dependencies]
 lalrpop.workspace = true
@@ -67,6 +68,8 @@ topiary = { workspace = true, optional = true }
 topiary-queries = { workspace = true, optional = true }
 tree-sitter-nickel = { workspace = true, optional = true }
 
+metrics = { workspace = true, optional = true }
+
 [dev-dependencies]
 pretty_assertions.workspace = true
 assert_matches.workspace = true

core/src/environment.rs

@@ -7,6 +7,8 @@ use std::marker::PhantomData;
 use std::ptr::NonNull;
 use std::rc::Rc;
 
+use crate::metrics::increment;
+
 /// An environment as a linked-list of hashmaps.
 ///
 /// Each node of the linked-list corresponds to what is called
@@ -38,6 +40,7 @@ impl<K: Hash + Eq, V: PartialEq> Clone for Environment<K, V> {
  /// and if it wasn't, it sets the `current` has the new head of `previous`.
  /// Then a clone is an empty `current` and the clone of `self.previous`.
  fn clone(&self) -> Self {
+ increment!("Environment::clone");
  if !self.current.is_empty() && !self.was_cloned() {
  self.previous.replace_with(|old| {
  Some(Rc::new(Environment {

core/src/identifier.rs

@@ -7,7 +7,7 @@ use std::{
  hash::Hash,
 };
 
-use crate::{position::TermPos, term::string::NickelString};
+use crate::{metrics::increment, position::TermPos, term::string::NickelString};
 
 simple_counter::generate_counter!(GeneratedCounter, usize);
 static INTERNER: Lazy<interner::Interner> = Lazy::new(interner::Interner::new);
@@ -126,6 +126,7 @@ impl LocIdent {
  /// with any identifier defined before. Generated identifiers start with a special prefix that
  /// can't be used by normal, user-defined identifiers.
  pub fn fresh() -> Self {
+ increment!("LocIdent::fresh");
  Self::new(format!("{}{}", GEN_PREFIX, GeneratedCounter::next()))
  }
 

core/src/lib.rs

@@ -19,5 +19,7 @@ pub mod transform;
 pub mod typ;
 pub mod typecheck;
 
+pub(crate) mod metrics;
+
 #[cfg(feature = "format")]
 pub mod format;

core/src/metrics.rs

@@ -0,0 +1,19 @@
+//! This module only contains macros wrapping those of the `metrics` crate, if
+//! the `metrics` feature is enabled.
+
+#[cfg(feature = "metrics")]
+macro_rules! increment {
+ ( $counter:expr ) => {
+ $crate::metrics::increment!($counter, 1)
+ };
+ ( $counter:expr, $count:expr ) => {
+ ::metrics::counter!($counter, $count)
+ };
+}
+
+#[cfg(not(feature = "metrics"))]
+macro_rules! increment {
+ ( $( $args:expr ),+ ) => {};
+}
+
+pub(crate) use increment;

core/src/program.rs

@@ -27,6 +27,7 @@ use crate::{
  eval::{cache::Cache as EvalCache, VirtualMachine},
  identifier::LocIdent,
  label::Label,
+ metrics::increment,
  term::{
  make as mk_term, make::builder, record::Field, BinaryOp, MergePriority, RichTerm, Term,
  },
@@ -181,6 +182,8 @@ impl<EC: EvalCache> Program<EC> {
  path: impl Into<OsString>,
  trace: impl Write + 'static,
  ) -> std::io::Result<Self> {
+ increment!("Program::new");
+
  let mut cache = Cache::new(ErrorTolerance::Strict);
  let main_id = cache.add_file(path)?;
  let vm = VirtualMachine::new(cache, trace);
@@ -202,6 +205,8 @@ impl<EC: EvalCache> Program<EC> {
  T: Read,
  S: Into<OsString> + Clone,
  {
+ increment!("Program::new");
+
  let mut cache = Cache::new(ErrorTolerance::Strict);
  let path = PathBuf::from(source_name.into());
  let main_id = cache.add_source(SourcePath::Path(path), source)?;