-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add benchmarks section to DEVELOPERS.md #1838
Conversation
@@ -229,7 +229,7 @@ fn criterion_benchmark(c: &mut Criterion) { | |||
}); | |||
} | |||
|
|||
// Clean up temporary file if any | |||
// Temporary file must outlive the benchmarks, it is deleted when dropped |
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 was review feedback from #1738 (comment)
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.
sorry I needed the dopamine hit of clicking the green button -- I should have waited
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.
Thank you @tustvold
cargo bench --bench parquet_query_sql | ||
``` | ||
|
||
These randomly generate a parquet file, and then benchmark queries sourced from [parquet_query_sql.sql](./datafusion/benches/parquet_query_sql.sql) against it. This can therefore be a quick way to add coverage of particular query and/or data paths. |
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.
👍
@@ -229,7 +229,7 @@ fn criterion_benchmark(c: &mut Criterion) { | |||
}); | |||
} | |||
|
|||
// Clean up temporary file if any | |||
// Temporary file must outlive the benchmarks, it is deleted when dropped |
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.
sorry I needed the dopamine hit of clicking the green button -- I should have waited
Rationale for this change
This was requested on #1738 (comment)
What changes are included in this PR?
Adds some documentation on how to run the criterion benchmarks
Are there any user-facing changes?
No code changes