-
Notifications
You must be signed in to change notification settings - Fork 123
fix(tests): Fix return values for stdlib benchmarks #1814
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?
Changes from all commits
19052f5
275c939
2d4bfdd
ef906a3
a561e73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,10 @@ macro_rules! bench_function { | |
| let tz = $crate::compiler::TimeZone::Named(chrono_tz::Tz::UTC); | ||
| let mut ctx = $crate::compiler::Context::new(&mut target, &mut runtime_state, &tz); | ||
|
|
||
| // Checks if function returns correct result before starting benchmarking | ||
| let got = expression.resolve(&mut ctx).map_err(|e| e.to_string()); | ||
| assert_eq!(got, want); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With this unconditional Useful? React with 👍 / 👎. |
||
|
|
||
| b.iter(|| { | ||
| let got = expression.resolve(&mut ctx).map_err(|e| e.to_string()); | ||
| debug_assert_eq!(got, want); | ||
|
|
@@ -83,6 +87,13 @@ macro_rules! bench_query_function { | |
| let tz = $crate::compiler::TimeZone::Named(chrono_tz::Tz::UTC); | ||
| let event: $crate::value::Value = $event.into(); | ||
|
|
||
| // Checks if function returns correct result before starting benchmarking | ||
| let mut runtime_state = $crate::compiler::state::RuntimeState::default(); | ||
| let mut target = event.clone(); | ||
| let mut ctx = $crate::compiler::Context::new(&mut target, &mut runtime_state, &tz); | ||
| let got = expression.resolve(&mut ctx).map_err(|e| e.to_string()); | ||
| assert_eq!(got, want); | ||
|
|
||
| b.iter_batched(|| { | ||
| let mut runtime_state = $crate::compiler::state::RuntimeState::default(); | ||
| let mut target = event.clone(); | ||
|
|
||
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 shared constant also feeds the
parse_regex/single_matchbenchmark, while the documented example and unit test still use(?P<number>.*?) groupinsrc/stdlib/parse_regex.rs. That old pattern already returns{"number":"first"}for the single-match case, so changing it here only alters the workload being measured (lazy wildcard vs. negated character class) and makes futureparse_regextimings incomparable with previous runs; use a separate pattern for theparse_regex_allcase if that benchmark needs different captures.Useful? React with 👍 / 👎.