Merge pull request #665 from mozilla/leave-it-open

Keep the stores open all the time

Cargo.toml

@@ -4,6 +4,7 @@ members = [
   "glean-core",
   "glean-core/ffi",
   "glean-core/preview",
+  "glean-core/benchmark",
 ]
 
 [profile.release]

Makefile

@@ -67,6 +67,13 @@ test-python: build-python ## Run all Python tests
 
 .PHONY: test test-rust test-rust-with-logs test-kotlin test-swift test-ios-sample
 
+# Benchmarks
+
+bench-rust: ## Run Rust benchmarks
+	cargo bench -p benchmark
+
+.PHONY: bench-rust
+
 # Linting
 
 lint: clippy

glean-core/benchmark/Cargo.toml

@@ -0,0 +1,24 @@
+[package]
+name = "benchmark"
+version = "0.1.0"
+authors = ["Jan-Erik Rediger <jrediger@mozilla.com>"]
+description = "Benchmark the glean-core crate"
+readme = "README.md"
+license = "MPL-2.0"
+edition = "2018"
+publish = false
+
+[lib]
+bench = false
+
+[dependencies]
+# No version specified, this crate never gets published
+glean-core = { path = ".." }
+
+[dev-dependencies]
+tempfile = "3.1.0"
+criterion = "0.3"
+
+[[bench]]
+name = "bench_basic"
+harness = false

glean-core/benchmark/README.md

@@ -0,0 +1,38 @@
+# Glean Benchmarks
+
+The `Glean SDK` is a modern approach for a Telemetry library and is part of the [Glean project](https://docs.telemetry.mozilla.org/concepts/glean/glean.html).
+
+## Benchmarks
+
+This crates provides simple benchmarks for Glean, based on the [criterion benchmark framework](https://bheisler.github.io/criterion.rs/book/criterion_rs.html).
+The library itself does not contain additional code.
+
+### Available benchmarks
+
+* [`benches/bench_basic.rs`](benches/bench_basic.rs) - Setting metrics and submitting a custom ping
+
+### How to run the benchmarks
+
+From the top-level directory of the repository run:
+
+```
+cargo bench -p benchmark
+```
+
+This is also available as a `make` task:
+
+```
+make bench-rust
+```
+
+Any additional run compares results to the preceding run.
+
+### Results
+
+After running the benchmarks, an HTML-rendered report can be found in `target/criterion/report/index.html`.
+We do not provide any results here as the absolute numbers are unreliable.
+Benchmark timings across code changes are more important.
+
+### Why an additional crate?
+
+This way we don't add any new (dev) dependencies to the crates that get released.

glean-core/benchmark/benches/bench_basic.rs

@@ -0,0 +1,48 @@
+use benchmark::glean_core::{metrics::*, CommonMetricData, Configuration, Glean};
+use criterion::{criterion_group, criterion_main, Criterion};
+
+/// Set metrics and submit a custom ping.
+///
+/// Glean, the metrics and the custom ping are instantiated before benchmarking the set/submit
+/// functionality.
+pub fn criterion_benchmark(c: &mut Criterion) {
+    let data_dir = tempfile::tempdir().unwrap();
+    let tmpname = data_dir.path().display().to_string();
+    let cfg = Configuration {
+        upload_enabled: true,
+        data_path: tmpname,
+        application_id: "glean.bench".into(),
+        max_events: None,
+        delay_ping_lifetime_io: false,
+    };
+
+    let mut glean = Glean::new(cfg).unwrap();
+
+    let ping = PingType::new("sample", true, false);
+    glean.register_ping_type(&ping);
+
+    let call_counter: CounterMetric = CounterMetric::new(CommonMetricData {
+        name: "calls".into(),
+        category: "local".into(),
+        send_in_pings: vec!["sample".into()],
+        ..Default::default()
+    });
+
+    let string: StringMetric = StringMetric::new(CommonMetricData {
+        name: "string".into(),
+        category: "local".into(),
+        send_in_pings: vec!["sample".into()],
+        ..Default::default()
+    });
+
+    c.bench_function("glean set and submit", |b| {
+        b.iter(|| {
+            call_counter.add(&glean, 1);
+            string.set(&glean, "hello world");
+            glean.submit_ping(&ping).unwrap();
+        })
+    });
+}
+
+criterion_group!(benches, criterion_benchmark);
+criterion_main!(benches);

glean-core/benchmark/src/lib.rs

@@ -0,0 +1,14 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! # Glean benchmarks
+//!
+//! Benchmarking the glean-core crate. Find benchmarks in the `benches/` directory.
+//!
+//! All documentation for Glean can be found online:
+//!
+//! ## [The Glean SDK Book](https://mozilla.github.io/glean)
+
+// Re-export glean_core in total.
+pub use glean_core;

glean-core/src/database/mod.rs

@@ -16,15 +16,36 @@ use crate::Glean;
 use crate::Lifetime;
 use crate::Result;
 
-#[derive(Debug)]
 pub struct Database {
+    /// Handle to the database environment.
     rkv: Rkv,
-    // If the `delay_ping_lifetime_io` Glean config option is `true`,
-    // we will save metrics with 'ping' lifetime data in a map temporarily
-    // so as to persist them to disk using rkv in bulk on demand.
+
+    /// Handles to the "lifetime" stores.
+    ///
+    /// A "store" is a handle to the underlying database.
+    /// We keep them open for fast and frequent access.
+    user_store: SingleStore,
+    ping_store: SingleStore,
+    application_store: SingleStore,
+
+    /// If the `delay_ping_lifetime_io` Glean config option is `true`,
+    /// we will save metrics with 'ping' lifetime data in a map temporarily
+    /// so as to persist them to disk using rkv in bulk on demand.
     ping_lifetime_data: Option<RwLock<BTreeMap<String, Metric>>>,
 }
 
+impl std::fmt::Debug for Database {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+        fmt.debug_struct("Database")
+            .field("rkv", &self.rkv)
+            .field("user_store", &"SingleStore")
+            .field("ping_store", &"SingleStore")
+            .field("application_store", &"SingleStore")
+            .field("ping_lifetime_data", &self.ping_lifetime_data)
+            .finish()
+    }
+}
+
 impl Database {
     /// Initialize the data store.
     ///
@@ -34,20 +55,38 @@ impl Database {
     /// It also loads any Lifetime::Ping data that might be
     /// persisted, in case `delay_ping_lifetime_io` is set.
     pub fn new(data_path: &str, delay_ping_lifetime_io: bool) -> Result<Self> {
+        let rkv = Self::open_rkv(data_path)?;
+        let user_store = rkv.open_single(Lifetime::User.as_str(), StoreOptions::create())?;
+        let ping_store = rkv.open_single(Lifetime::Ping.as_str(), StoreOptions::create())?;
+        let application_store =
+            rkv.open_single(Lifetime::Application.as_str(), StoreOptions::create())?;
+        let ping_lifetime_data = if delay_ping_lifetime_io {
+            Some(RwLock::new(BTreeMap::new()))
+        } else {
+            None
+        };
+
         let db = Self {
-            rkv: Self::open_rkv(data_path)?,
-            ping_lifetime_data: if delay_ping_lifetime_io {
-                Some(RwLock::new(BTreeMap::new()))
-            } else {
-                None
-            },
+            rkv,
+            user_store,
+            ping_store,
+            application_store,
+            ping_lifetime_data,
         };
 
         db.load_ping_lifetime_data();
 
         Ok(db)
     }
 
+    fn get_store(&self, lifetime: Lifetime) -> &SingleStore {
+        match lifetime {
+            Lifetime::User => &self.user_store,
+            Lifetime::Ping => &self.ping_store,
+            Lifetime::Application => &self.application_store,
+        }
+    }
+
     /// Creates the storage directories and inits rkv.
     fn open_rkv(path: &str) -> Result<Rkv> {
         let path = std::path::Path::new(path).join("db");
@@ -89,13 +128,8 @@ impl Database {
                 .write()
                 .expect("Can't read ping lifetime data");
 
-            let store: SingleStore = unwrap_or!(
-                self.rkv
-                    .open_single(Lifetime::Ping.as_str(), StoreOptions::create()),
-                return
-            );
-
             let reader = unwrap_or!(self.rkv.read(), return);
+            let store = self.get_store(Lifetime::Ping);
             let mut iter = unwrap_or!(store.iter_start(&reader), return);
 
             while let Some(Ok((metric_name, value))) = iter.next() {
@@ -164,15 +198,12 @@ impl Database {
             }
         }
 
-        let store: SingleStore = unwrap_or!(
-            self.rkv
-                .open_single(lifetime.as_str(), StoreOptions::create()),
+        let reader = unwrap_or!(self.rkv.read(), return);
+        let mut iter = unwrap_or!(
+            self.get_store(lifetime).iter_from(&reader, &iter_start),
             return
         );
 
-        let reader = unwrap_or!(self.rkv.read(), return);
-        let mut iter = unwrap_or!(store.iter_from(&reader, &iter_start), return);
-
         while let Some(Ok((metric_name, value))) = iter.next() {
             if !metric_name.starts_with(iter_start.as_bytes()) {
                 break;
@@ -219,13 +250,11 @@ impl Database {
             }
         }
 
-        let store: SingleStore = unwrap_or!(
-            self.rkv
-                .open_single(lifetime.as_str(), StoreOptions::create()),
-            return false
-        );
         let reader = unwrap_or!(self.rkv.read(), return false);
-        store.get(&reader, &key).unwrap_or(None).is_some()
+        self.get_store(lifetime)
+            .get(&reader, &key)
+            .unwrap_or(None)
+            .is_some()
     }
 
     /// Write to the specified storage with the provided transaction function.
@@ -237,14 +266,11 @@ impl Database {
     /// * This function will **not** panic on database errors.
     pub fn write_with_store<F>(&self, store_name: Lifetime, mut transaction_fn: F) -> Result<()>
     where
-        F: FnMut(rkv::Writer, SingleStore) -> Result<()>,
+        F: FnMut(rkv::Writer, &SingleStore) -> Result<()>,
     {
-        let store: SingleStore = self
-            .rkv
-            .open_single(store_name.as_str(), StoreOptions::create())?;
-        let writer = self.rkv.write()?;
-        transaction_fn(writer, store)?;
-        Ok(())
+        let writer = self.rkv.write().unwrap();
+        let store = self.get_store(store_name);
+        transaction_fn(writer, store)
     }
 
     /// Records a metric in the underlying storage system.
@@ -293,11 +319,9 @@ impl Database {
         let encoded = bincode::serialize(&metric).expect("IMPOSSIBLE: Serializing metric failed");
         let value = rkv::Value::Blob(&encoded);
 
-        let store_name = lifetime.as_str();
-        let store = self.rkv.open_single(store_name, StoreOptions::create())?;
-
         let mut writer = self.rkv.write()?;
-        store.put(&mut writer, final_key, &value)?;
+        self.get_store(lifetime)
+            .put(&mut writer, final_key, &value)?;
         writer.commit()?;
         Ok(())
     }
@@ -363,10 +387,8 @@ impl Database {
             }
         }
 
-        let store_name = lifetime.as_str();
-        let store = self.rkv.open_single(store_name, StoreOptions::create())?;
-
         let mut writer = self.rkv.write()?;
+        let store = self.get_store(lifetime);
         let new_value: Metric = {
             let old_value = store.get(&writer, &final_key)?;