Skip to content

Commit f956c04

Browse files
authored
fix: benchmark names with multiple lines (#620)
1 parent d84683b commit f956c04

File tree

3 files changed

+216
-6
lines changed

3 files changed

+216
-6
lines changed

lib/bencher_adapter/src/adapters/rust/iai_callgrind.rs

Lines changed: 143 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ use bencher_json::{
1010
use nom::{
1111
IResult,
1212
branch::alt,
13-
bytes::complete::{is_a, is_not, tag},
13+
bytes::complete::{is_a, is_not, tag, take_until1},
1414
character::complete::{char, space0, space1},
15-
combinator::{map, opt, recognize},
15+
combinator::{map, opt, peek, recognize},
1616
multi::{many0, many1},
1717
sequence::{delimited, preceded, terminated, tuple},
1818
};
@@ -76,11 +76,11 @@ fn single_benchmark<'a>()
7676
recognize(tuple((
7777
is_not(":\r\n \t"),
7878
tag("::"),
79-
is_not(":\r\n"),
79+
is_not(":\r\n \t"),
8080
tag("::"),
81-
not_line_ending(),
81+
not_benchmark_name_end(),
8282
))),
83-
line_ending(),
83+
benchmark_name_end(),
8484
),
8585
many0(alt((
8686
// Callgrind tool if it was enabled:
@@ -98,7 +98,9 @@ fn single_benchmark<'a>()
9898
))),
9999
)),
100100
|(benchmark_name, measures)| {
101-
let benchmark_name = benchmark_name.parse().ok()?;
101+
// trim here to avoid loose `\r` chars at the end of the string because of the
102+
// `not_benchmark_name_end` parser. It's maybe not a bad idea anyways.
103+
let benchmark_name = benchmark_name.trim().parse().ok()?;
102104
let measures = measures.into_iter().flatten().collect();
103105

104106
Some((benchmark_name, measures))
@@ -483,6 +485,24 @@ fn not_line_ending<'a>() -> impl FnMut(&'a str) -> IResult<&'a str, &'a str> {
483485
is_not("\r\n")
484486
}
485487

488+
/// Parser that matches the benchmark name ending sequence, excluding the two spaces
489+
fn benchmark_name_end<'a>() -> impl FnMut(&'a str) -> IResult<&'a str, (&'a str, &'a str)> {
490+
// we only peek here so the `metric_line` parser can match the first spaces too
491+
tuple((line_ending(), peek(tag(" "))))
492+
}
493+
494+
/// A parser that matches input until it encounters the benchmark name end sequence.
495+
///
496+
/// Similar to the `benchmark_name_end` parser, this parser addresses an unusual behavior in
497+
/// `gungraun` (iai-callgrind) by allowing benchmark names to span multiple lines. While this
498+
/// behavior is not a bug in `gungraun`, it deviates from the original intent. If there are multiple
499+
/// lines they don't start with two spaces, so we can use that as test for the end of the benchmark
500+
/// name.
501+
fn not_benchmark_name_end<'a>() -> impl FnMut(&'a str) -> IResult<&'a str, &'a str> {
502+
// Ignore the `\r\n` possibility here and instead trim the benchmark name later
503+
take_until1("\n ")
504+
}
505+
486506
#[cfg(test)]
487507
pub(crate) mod test_rust_iai_callgrind {
488508
use crate::{AdapterResults, adapters::test_util::convert_file_path};
@@ -1045,6 +1065,123 @@ pub(crate) mod test_rust_iai_callgrind {
10451065
}
10461066
}
10471067

1068+
#[test]
1069+
fn test_name_multiple_lines() {
1070+
use iai_callgrind::*;
1071+
1072+
let results = convert_file_path::<AdapterRustIaiCallgrind>(
1073+
"./tool_output/rust/iai_callgrind/name_on_multiple_lines.txt",
1074+
);
1075+
1076+
assert_eq!(results.inner.len(), 2);
1077+
1078+
{
1079+
let mut expected = HashMap::new();
1080+
1081+
expected.extend([
1082+
(Instructions::SLUG_STR, 1.0),
1083+
(L1Hits::SLUG_STR, 2.0),
1084+
(L2Hits::SLUG_STR, 3.0),
1085+
(RamHits::SLUG_STR, 4.0),
1086+
(TotalReadWrite::SLUG_STR, 5.0),
1087+
(EstimatedCycles::SLUG_STR, 6.0),
1088+
]);
1089+
1090+
compare_benchmark(
1091+
&expected,
1092+
&results,
1093+
"rust_iai_callgrind::bench::multiple_lines id:string with two\nlines",
1094+
);
1095+
}
1096+
1097+
{
1098+
let mut expected = HashMap::new();
1099+
1100+
expected.extend([
1101+
(Instructions::SLUG_STR, 7.0),
1102+
(L1Hits::SLUG_STR, 8.0),
1103+
(L2Hits::SLUG_STR, 9.0),
1104+
(RamHits::SLUG_STR, 10.0),
1105+
(TotalReadWrite::SLUG_STR, 11.0),
1106+
(EstimatedCycles::SLUG_STR, 12.0),
1107+
]);
1108+
1109+
compare_benchmark(
1110+
&expected,
1111+
&results,
1112+
"rust_iai_callgrind::bench::multiple_lines id:string with multiple\nlines\nand one more",
1113+
);
1114+
}
1115+
}
1116+
1117+
#[test]
1118+
fn test_name_multiple_lines_mixed() {
1119+
use iai_callgrind::*;
1120+
1121+
let results = convert_file_path::<AdapterRustIaiCallgrind>(
1122+
"./tool_output/rust/iai_callgrind/name_on_multiple_lines_mixed.txt",
1123+
);
1124+
1125+
assert_eq!(results.inner.len(), 3);
1126+
1127+
{
1128+
let mut expected = HashMap::new();
1129+
1130+
expected.extend([
1131+
(Instructions::SLUG_STR, 1.0),
1132+
(L1Hits::SLUG_STR, 2.0),
1133+
(L2Hits::SLUG_STR, 3.0),
1134+
(RamHits::SLUG_STR, 4.0),
1135+
(TotalReadWrite::SLUG_STR, 5.0),
1136+
(EstimatedCycles::SLUG_STR, 6.0),
1137+
]);
1138+
1139+
compare_benchmark(
1140+
&expected,
1141+
&results,
1142+
"rust_iai_callgrind::bench::multiple_lines id:first with one line",
1143+
);
1144+
}
1145+
1146+
{
1147+
let mut expected = HashMap::new();
1148+
1149+
expected.extend([
1150+
(Instructions::SLUG_STR, 7.0),
1151+
(L1Hits::SLUG_STR, 8.0),
1152+
(L2Hits::SLUG_STR, 9.0),
1153+
(RamHits::SLUG_STR, 10.0),
1154+
(TotalReadWrite::SLUG_STR, 11.0),
1155+
(EstimatedCycles::SLUG_STR, 12.0),
1156+
]);
1157+
1158+
compare_benchmark(
1159+
&expected,
1160+
&results,
1161+
"rust_iai_callgrind::bench::multiple_lines id:two\nlines",
1162+
);
1163+
}
1164+
1165+
{
1166+
let mut expected = HashMap::new();
1167+
1168+
expected.extend([
1169+
(Instructions::SLUG_STR, 13.0),
1170+
(L1Hits::SLUG_STR, 14.0),
1171+
(L2Hits::SLUG_STR, 15.0),
1172+
(RamHits::SLUG_STR, 16.0),
1173+
(TotalReadWrite::SLUG_STR, 17.0),
1174+
(EstimatedCycles::SLUG_STR, 18.0),
1175+
]);
1176+
1177+
compare_benchmark(
1178+
&expected,
1179+
&results,
1180+
"rust_iai_callgrind::bench::multiple_lines id:last with one line",
1181+
);
1182+
}
1183+
}
1184+
10481185
#[derive(Default)]
10491186
pub struct OptionalMetrics {
10501187
pub global_bus_events: bool,
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
2+
running 0 tests
3+
4+
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
5+
6+
rust_iai_callgrind::bench::multiple_lines id:string with two
7+
lines
8+
Instructions: 1|N/A (*********)
9+
L1 Hits: 2|N/A (*********)
10+
L2 Hits: 3|N/A (*********)
11+
RAM Hits: 4|N/A (*********)
12+
Total read+write: 5|N/A (*********)
13+
Estimated Cycles: 6|N/A (*********)
14+
rust_iai_callgrind::bench::multiple_lines id:string with multiple
15+
lines
16+
and one more
17+
Instructions: 7|N/A (*********)
18+
L1 Hits: 8|N/A (*********)
19+
L2 Hits: 9|N/A (*********)
20+
RAM Hits: 10|N/A (*********)
21+
Total read+write: 11|N/A (*********)
22+
Estimated Cycles: 12|N/A (*********)
23+
24+
25+
26+
27+
28+
29+
30+
31+
32+
.... Some trailing text that should be ignored ...
33+
.... Some trailing text that should be ignored ...
34+
.... Some trailing text that should be ignored ...
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
2+
running 0 tests
3+
4+
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
5+
6+
rust_iai_callgrind::bench::multiple_lines id:first with one line
7+
Instructions: 1|N/A (*********)
8+
L1 Hits: 2|N/A (*********)
9+
L2 Hits: 3|N/A (*********)
10+
RAM Hits: 4|N/A (*********)
11+
Total read+write: 5|N/A (*********)
12+
Estimated Cycles: 6|N/A (*********)
13+
rust_iai_callgrind::bench::multiple_lines id:two
14+
lines
15+
Instructions: 7|N/A (*********)
16+
L1 Hits: 8|N/A (*********)
17+
L2 Hits: 9|N/A (*********)
18+
RAM Hits: 10|N/A (*********)
19+
Total read+write: 11|N/A (*********)
20+
Estimated Cycles: 12|N/A (*********)
21+
rust_iai_callgrind::bench::multiple_lines id:last with one line
22+
Instructions: 13|N/A (*********)
23+
L1 Hits: 14|N/A (*********)
24+
L2 Hits: 15|N/A (*********)
25+
RAM Hits: 16|N/A (*********)
26+
Total read+write: 17|N/A (*********)
27+
Estimated Cycles: 18|N/A (*********)
28+
29+
30+
31+
32+
33+
34+
35+
36+
37+
.... Some trailing text that should be ignored ...
38+
.... Some trailing text that should be ignored ...
39+
.... Some trailing text that should be ignored ...

0 commit comments

Comments
 (0)