-
Notifications
You must be signed in to change notification settings - Fork 3
Add basic coin-store bench #52
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
base: main
Are you sure you want to change the base?
Conversation
Greptile SummaryAdds Criterion-based benchmarks for the coin-store crate to measure UTXO query performance under different filter configurations (default, entropy, contracts, token tags). The PR includes:
The benchmarks follow a standard pattern: setup database with test data, run queries with Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant B as Benchmark Suite
participant C as common::setup_db()
participant S as Store
participant DB as SQLite Database
B->>C: Initialize benchmark
C->>DB: Remove old /tmp/benchmark_stress.db
C->>S: Store::create()
S->>DB: Create tables and run migrations
loop 10 iterations
C->>C: Generate keypair (same key)
C->>C: setup_tx_and_contract()
C->>C: Generate random entropy
C->>C: Build option creation tx
C->>S: insert_transaction(tx, blinder_keys)
S->>DB: Mark inputs as spent (noop)
S->>DB: Insert UTXOs
C->>S: add_contract(source, args, tpg)
S->>DB: Store contract metadata
C->>S: insert_contract_token(tpg, asset_id)
S->>DB: Store token mapping
C->>C: Build filters (default, entropy, contracts, token)
end
C-->>B: Return (store, filters, db_path)
loop Benchmark iterations
B->>S: query_utxos(filters)
S->>DB: Execute SQL queries with filters
DB-->>S: Return matching UTXOs
S-->>B: UtxoQueryResult
end
B->>DB: Remove /tmp/benchmark_stress.db
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, 3 comments
|
|
||
| for _i in 0..10 { | ||
| let secp = Secp256k1::new(); | ||
| let keypair = Keypair::from_secret_key(&secp, &SecretKey::from_slice(&[1u8; 32]).unwrap()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: same keypair used in every iteration - all UTXOs will have identical blinding keys, which may not accurately represent real-world query performance with diverse data
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/coin-store/benches/common.rs
Line: 95:95
Comment:
**style:** same keypair used in every iteration - all UTXOs will have identical blinding keys, which may not accurately represent real-world query performance with diverse data
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main mission of thins fn is to set up a database for query testing, and in my opinion, it does not affect query processing time.
| store | ||
| .insert_contract_token(&tpg_for_token.clone(), option_asset_id, "Name of token") | ||
| .await | ||
| .expect(""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: empty error message won't help debug if insert_contract_token fails
| .expect(""); | |
| .expect("Failed to insert contract token"); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/coin-store/benches/common.rs
Line: 122:122
Comment:
**style:** empty error message won't help debug if `insert_contract_token` fails
```suggestion
.expect("Failed to insert contract token");
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my mistake, I will fix it.its my bad, i will fix that
| (Vec<UtxoFilter>, Vec<UtxoFilter>, Vec<UtxoFilter>, Vec<UtxoFilter>), | ||
| String, | ||
| ) { | ||
| let path = "/tmp/benchmark_stress.db"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: hardcoded database path could cause conflicts if multiple benchmarks run concurrently - consider using a unique path per benchmark or test run
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/coin-store/benches/common.rs
Line: 84:84
Comment:
**style:** hardcoded database path could cause conflicts if multiple benchmarks run concurrently - consider using a unique path per benchmark or test run
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the main way to optimize a database is to configure it to use one (possibly large) function and only configure filters. So if I have one database with a dataset, I don't need to store it in different places.
| Store, | ||
| (Vec<UtxoFilter>, Vec<UtxoFilter>, Vec<UtxoFilter>, Vec<UtxoFilter>), | ||
| String, | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's Result here to avoid unwraps
|
|
||
| mod common; | ||
|
|
||
| fn criterion_benchmark(c: &mut Criterion) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to make this function async?
No description provided.