Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the stores open all the time #665

Merged
merged 2 commits into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
313 changes: 313 additions & 0 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ members = [
"glean-core",
"glean-core/ffi",
"glean-core/preview",
"glean-core/benchmark",
]

[profile.release]
Expand Down
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions glean-core/benchmark/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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
373 changes: 373 additions & 0 deletions glean-core/benchmark/LICENSE

Large diffs are not rendered by default.

38 changes: 38 additions & 0 deletions glean-core/benchmark/README.md
Original file line number Diff line number Diff line change
@@ -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.
48 changes: 48 additions & 0 deletions glean-core/benchmark/benches/bench_basic.rs
Original file line number Diff line number Diff line change
@@ -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);
14 changes: 14 additions & 0 deletions glean-core/benchmark/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
106 changes: 64 additions & 42 deletions glean-core/src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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");
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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)?;

Expand Down