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

internal: add some initial input benchmarks #525

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

davidbarsky
Copy link
Contributor

Pulling from https:/davidbarsky/salsa-benchmarks/; these should probably live in here.

Copy link

netlify bot commented Jul 23, 2024

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 0557605
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/66bb6ff920f9dd000838e444

Copy link
Member

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look fine -- will they be integrated into codspeed, @MichaReiser, or do we have to take additional steps?

Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should be picked up automatically after rebasing on master and using codspeed_criterion_compat::* instead of criterion::*.

@MichaReiser
Copy link
Contributor

Patch to integrate it with codspeed / new db design

Subject: [PATCH] Use codspeed
---
Index: Cargo.toml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Cargo.toml b/Cargo.toml
--- a/Cargo.toml	(revision 892862325c0f58fe61b138413be2907367c7481c)
+++ b/Cargo.toml	(revision 171e116fcc50b0d2c1937fa84ab1e02d49fd9151)
@@ -34,7 +34,6 @@
 rustversion = "1.0"
 test-log = "0.2.11"
 trybuild = "1.0"
-criterion = "0.5.1"
 
 [[bench]]
 name = "compare"
Index: benches/compare.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/benches/compare.rs b/benches/compare.rs
--- a/benches/compare.rs	(revision 892862325c0f58fe61b138413be2907367c7481c)
+++ b/benches/compare.rs	(revision 171e116fcc50b0d2c1937fa84ab1e02d49fd9151)
@@ -1,28 +1,13 @@
-use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
+use codspeed_criterion_compat::{criterion_group, criterion_main, BenchmarkId, Criterion};
 use salsa::Setter;
 
-#[salsa::db]
-pub trait Db: salsa::Database {}
-
-#[salsa::db]
-#[derive(Default)]
-pub struct Database {
-    storage: salsa::Storage<Self>,
-}
-
-#[salsa::db]
-impl salsa::Database for Database {}
-
-#[salsa::db]
-impl Db for Database {}
-
 #[salsa::input]
 pub struct Input {
     pub text: String,
 }
 
 #[salsa::tracked]
-pub fn length(db: &dyn Db, input: Input) -> usize {
+pub fn length(db: &dyn salsa::Database, input: Input) -> usize {
     input.text(db).len()
 }
 
@@ -32,15 +17,16 @@
 }
 
 #[salsa::tracked]
-pub fn interned_length<'db>(db: &'db dyn Db, input: InternedInput<'db>) -> usize {
+pub fn interned_length<'db>(db: &'db dyn salsa::Database, input: InternedInput<'db>) -> usize {
     input.text(db).len()
 }
 
 fn mutating_inputs(c: &mut Criterion) {
-    let mut group: criterion::BenchmarkGroup<criterion::measurement::WallTime> =
-        c.benchmark_group("Mutating Inputs");
+    let mut group: codspeed_criterion_compat::BenchmarkGroup<
+        codspeed_criterion_compat::measurement::WallTime,
+    > = c.benchmark_group("Mutating Inputs");
 
-    let mut db = Database::default();
+    let mut db = salsa::DatabaseImpl::default();
 
     for n in &[10, 20, 30] {
         let base_string = "hello, world!".to_owned();
@@ -68,9 +54,11 @@
 }
 
 fn inputs(c: &mut Criterion) {
-    let mut group: criterion::BenchmarkGroup<criterion::measurement::WallTime> =
-        c.benchmark_group("Inputs");
-    let db = Database::default();
+    let mut group: codspeed_criterion_compat::BenchmarkGroup<
+        codspeed_criterion_compat::measurement::WallTime,
+    > = c.benchmark_group("Mutating Inputs");
+
+    let db = salsa::DatabaseImpl::default();
 
     group.bench_function(BenchmarkId::new("new", "InternedInput"), |b| {
         b.iter(|| {

Copy link

codspeed-hq bot commented Aug 12, 2024

CodSpeed Performance Report

Merging #525 will not alter performance

Comparing davidbarsky:master (0557605) with master (4657ac3)

Summary

✅ 1 untouched benchmarks

🆕 7 new benchmarks

Benchmarks breakdown

Benchmark master davidbarsky:master Change
🆕 amortized[Input] N/A 3.9 µs N/A
🆕 amortized[InternedInput] N/A 3.3 µs N/A
🆕 new[Input] N/A 14 µs N/A
🆕 new[InternedInput] N/A 5.8 µs N/A
🆕 mutating[10] N/A 19.8 µs N/A
🆕 mutating[20] N/A 19 µs N/A
🆕 mutating[30] N/A 20.3 µs N/A

@davidbarsky
Copy link
Contributor Author

The test failure is due to rust-lang/rust#129031; I'll add an allow in the example.

@davidbarsky davidbarsky added this pull request to the merge queue Aug 13, 2024
Merged via the queue into salsa-rs:master with commit 08820ea Aug 13, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants