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

Make benchmark comparison more accurate on OSX #929

Merged
merged 2 commits into from
Jan 1, 2025
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
81 changes: 72 additions & 9 deletions benches/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use sanakirja::{Commit, RootDb};
use std::fs;
use std::fs::File;
use std::ops::Bound;
use std::path::Path;
use std::path::{Path, PathBuf};

#[allow(dead_code)]
const X: TableDefinition<&[u8], &[u8]> = TableDefinition::new("x");
Expand Down Expand Up @@ -360,7 +360,11 @@ impl BenchDatabase for HeedBenchDatabase {
fn write_transaction(&self) -> Self::W<'_> {
let env = self.env.as_ref().unwrap();
let txn = env.write_txn().unwrap();
Self::W { db: self.db, txn }
Self::W {
db: self.db,
db_dir: self.env.as_ref().unwrap().path().to_path_buf(),
txn,
}
}

fn read_transaction(&self) -> Self::R<'_> {
Expand All @@ -377,6 +381,7 @@ impl BenchDatabase for HeedBenchDatabase {
let file = env
.copy_to_file(path.join("data2.mdb"), CompactionOption::Enabled)
.unwrap();
file.sync_all().unwrap();
drop(file);

// We close the env
Expand Down Expand Up @@ -404,6 +409,8 @@ impl BenchDatabase for HeedBenchDatabase {

pub struct HeedBenchWriteTransaction<'db> {
db: heed::Database<heed::types::Bytes, heed::types::Bytes>,
#[allow(dead_code)]
db_dir: PathBuf,
txn: heed::RwTxn<'db>,
}

Expand All @@ -418,7 +425,21 @@ impl<'db> BenchWriteTransaction for HeedBenchWriteTransaction<'db> {
}

fn commit(self) -> Result<(), ()> {
self.txn.commit().map_err(|_| ())
let result = self.txn.commit().map_err(|_| ());
#[cfg(target_os = "macos")]
{
// Workaround for broken durability on MacOS in lmdb
// See: https://github.com/cberner/redb/pull/928#issuecomment-2567032808
for entry in fs::read_dir(self.db_dir).unwrap() {
let entry = entry.unwrap();
if entry.path().is_file() {
let file = File::open(entry.path()).unwrap();
file.sync_all().unwrap();
}
}
}

result
}
}

Expand Down Expand Up @@ -514,7 +535,10 @@ impl<'a> BenchDatabase for RocksdbBenchDatabase<'a> {
let mut txn_opt = OptimisticTransactionOptions::new();
txn_opt.set_snapshot(true);
let txn = self.db.transaction_opt(&write_opt, &txn_opt);
RocksdbBenchWriteTransaction { txn }
RocksdbBenchWriteTransaction {
txn,
db_dir: self.db.path().to_path_buf(),
}
}

fn read_transaction(&self) -> Self::R<'_> {
Expand All @@ -530,6 +554,8 @@ impl<'a> BenchDatabase for RocksdbBenchDatabase<'a> {

pub struct RocksdbBenchWriteTransaction<'a> {
txn: rocksdb::Transaction<'a, OptimisticTransactionDB>,
#[allow(dead_code)]
db_dir: PathBuf,
}

impl<'a> BenchWriteTransaction for RocksdbBenchWriteTransaction<'a> {
Expand All @@ -540,7 +566,21 @@ impl<'a> BenchWriteTransaction for RocksdbBenchWriteTransaction<'a> {
}

fn commit(self) -> Result<(), ()> {
self.txn.commit().map_err(|_| ())
let result = self.txn.commit().map_err(|_| ());
#[cfg(target_os = "macos")]
{
// Workaround for broken durability on MacOS in rocksdb
// See: https://github.com/cberner/redb/pull/928#issuecomment-2567032808
for entry in fs::read_dir(self.db_dir).unwrap() {
let entry = entry.unwrap();
if entry.path().is_file() {
let file = File::open(entry.path()).unwrap();
file.sync_all().unwrap();
}
}
}

result
}
}

Expand Down Expand Up @@ -614,11 +654,12 @@ impl BenchIterator for RocksdbBenchIterator<'_> {

pub struct SanakirjaBenchDatabase<'a> {
db: &'a sanakirja::Env,
db_dir: PathBuf,
}

impl<'a> SanakirjaBenchDatabase<'a> {
#[allow(dead_code)]
pub fn new(db: &'a sanakirja::Env) -> Self {
pub fn new(db: &'a sanakirja::Env, path: &'_ Path) -> Self {
let mut txn = sanakirja::Env::mut_txn_begin(db).unwrap();
// XXX: There's no documentation on why this method is unsafe, so let's just hope we upheld the requirements for it to be safe!
let table = unsafe {
Expand All @@ -627,7 +668,10 @@ impl<'a> SanakirjaBenchDatabase<'a> {
};
txn.set_root(0, table.db.into());
txn.commit().unwrap();
Self { db }
Self {
db,
db_dir: path.to_path_buf(),
}
}
}

Expand All @@ -641,7 +685,10 @@ impl<'a> BenchDatabase for SanakirjaBenchDatabase<'a> {

fn write_transaction(&self) -> Self::W<'_> {
let txn = sanakirja::Env::mut_txn_begin(self.db).unwrap();
SanakirjaBenchWriteTransaction { txn }
SanakirjaBenchWriteTransaction {
txn,
db_dir: self.db_dir.clone(),
}
}

fn read_transaction(&self) -> Self::R<'_> {
Expand All @@ -652,6 +699,8 @@ impl<'a> BenchDatabase for SanakirjaBenchDatabase<'a> {

pub struct SanakirjaBenchWriteTransaction<'db> {
txn: sanakirja::MutTxn<&'db sanakirja::Env, ()>,
#[allow(dead_code)]
db_dir: PathBuf,
}

impl<'db> BenchWriteTransaction for SanakirjaBenchWriteTransaction<'db> {
Expand All @@ -666,7 +715,21 @@ impl<'db> BenchWriteTransaction for SanakirjaBenchWriteTransaction<'db> {
}

fn commit(self) -> Result<(), ()> {
self.txn.commit().map_err(|_| ())
let result = self.txn.commit().map_err(|_| ());
#[cfg(target_os = "macos")]
{
// Workaround for presumably broken durability on MacOS
// See: https://github.com/cberner/redb/pull/928#issuecomment-2567032808
for entry in fs::read_dir(self.db_dir).unwrap() {
let entry = entry.unwrap();
if entry.path().is_file() {
let file = File::open(entry.path()).unwrap();
file.sync_all().unwrap();
}
}
}

result
}
}

Expand Down
2 changes: 1 addition & 1 deletion benches/int_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ fn main() {
let tmpfile: NamedTempFile = NamedTempFile::new_in(current_dir().unwrap()).unwrap();
fs::remove_file(tmpfile.path()).unwrap();
let db = sanakirja::Env::new(tmpfile.path(), 4096 * 1024 * 1024, 2).unwrap();
let table = SanakirjaBenchDatabase::new(&db);
let table = SanakirjaBenchDatabase::new(&db, tmpfile.path());
benchmark(table)
};

Expand Down
2 changes: 1 addition & 1 deletion benches/lmdb_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ fn main() {
let tmpfile: NamedTempFile = NamedTempFile::new_in(&tmpdir).unwrap();
fs::remove_file(tmpfile.path()).unwrap();
let db = sanakirja::Env::new(tmpfile.path(), 4096 * 1024 * 1024, 2).unwrap();
let table = SanakirjaBenchDatabase::new(&db);
let table = SanakirjaBenchDatabase::new(&db, &tmpdir);
benchmark(table, tmpfile.path())
};

Expand Down
Loading