Skip to content

Commit

Permalink
refactor: remove conditional color code in bench reporter (#23593)
Browse files Browse the repository at this point in the history 
There is no need for this conditional code because it's handled by the
`colors` module.
  • Loading branch information
@dsherret
dsherret committed Apr 29, 2024
1 parent 0b45405 commit 7d93704
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 124 deletions.
171 changes: 53 additions & 118 deletions cli/tools/bench/mitata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ pub mod reporter {
pub struct Options {
size: usize,
pub avg: bool,
pub colors: bool,
pub min_max: bool,
pub percentiles: bool,
}
Expand All @@ -210,7 +209,6 @@ pub mod reporter {
pub fn new(names: &[&str]) -> Options {
Options {
avg: true,
colors: true,
min_max: true,
size: size(names),
percentiles: true,
Expand Down Expand Up @@ -253,23 +251,12 @@ pub mod reporter {
let mut s = String::new();

s.push_str(&format!("{:<size$}", n));
s.push_str(&format!(
"{}: {}",
&(if !options.colors {
"error".to_string()
} else {
colors::red("error").to_string()
}),
e.message
));
s.push_str(&format!("{}: {}", colors::red("error"), e.message));

if let Some(ref stack) = e.stack {
s.push('\n');

match options.colors {
false => s.push_str(stack),
true => s.push_str(&colors::gray(stack).to_string()),
}
s.push_str(&colors::gray(stack).to_string());
}

s
Expand Down Expand Up @@ -304,64 +291,36 @@ pub mod reporter {

s.push_str(&format!("{:<size$}", name));

if !options.colors {
if options.avg {
s.push_str(&format!(
"{:>14}",
format!("{}/iter", fmt_duration(stats.avg))
));
s.push_str(&format!("{:>14}", avg_to_iter_per_s(stats.avg)));
}
if options.min_max {
s.push_str(&format!(
"{:>24}",
format!(
"({} … {})",
fmt_duration(stats.min),
fmt_duration(stats.max)
)
));
}
if options.percentiles {
s.push_str(&format!(
" {:>9} {:>9} {:>9}",
fmt_duration(stats.p75),
fmt_duration(stats.p99),
fmt_duration(stats.p995)
));
}
} else {
if options.avg {
s.push_str(&format!(
"{:>30}",
format!("{}/iter", colors::yellow(fmt_duration(stats.avg)))
));
s.push_str(&format!("{:>14}", avg_to_iter_per_s(stats.avg)));
}
if options.min_max {
s.push_str(&format!(
"{:>50}",
format!(
"({} … {})",
colors::cyan(fmt_duration(stats.min)),
colors::magenta(fmt_duration(stats.max))
)
));
}
if options.percentiles {
s.push_str(&format!(
" {:>22} {:>22} {:>22}",
colors::magenta(fmt_duration(stats.p75)),
colors::magenta(fmt_duration(stats.p99)),
colors::magenta(fmt_duration(stats.p995))
));
}
if options.avg {
s.push_str(&format!(
"{:>30}",
format!("{}/iter", colors::yellow(fmt_duration(stats.avg)))
));
s.push_str(&format!("{:>14}", avg_to_iter_per_s(stats.avg)));
}
if options.min_max {
s.push_str(&format!(
"{:>50}",
format!(
"({} … {})",
colors::cyan(fmt_duration(stats.min)),
colors::magenta(fmt_duration(stats.max))
)
));
}
if options.percentiles {
s.push_str(&format!(
" {:>22} {:>22} {:>22}",
colors::magenta(fmt_duration(stats.p75)),
colors::magenta(fmt_duration(stats.p99)),
colors::magenta(fmt_duration(stats.p995))
));
}

s
}

pub fn summary(benchmarks: &[GroupBenchmark], options: &Options) -> String {
pub fn summary(benchmarks: &[GroupBenchmark]) -> String {
let mut s = String::new();
let mut benchmarks = benchmarks.to_owned();
benchmarks.sort_by(|a, b| a.stats.avg.partial_cmp(&b.stats.avg).unwrap());
Expand All @@ -370,58 +329,34 @@ pub mod reporter {
.find(|b| b.baseline)
.unwrap_or(&benchmarks[0]);

if !options.colors {
s.push_str(&format!("summary\n {}", baseline.name));

for b in benchmarks.iter().filter(|b| *b != baseline) {
let faster = b.stats.avg >= baseline.stats.avg;
let diff = f64::from_str(&format!(
"{:.2}",
1.0 / baseline.stats.avg * b.stats.avg
))
.unwrap();
let inv_diff = f64::from_str(&format!(
"{:.2}",
1.0 / b.stats.avg * baseline.stats.avg
))
.unwrap();
s.push_str(&format!(
"\n {}x times {} than {}",
if faster { diff } else { inv_diff },
if faster { "faster" } else { "slower" },
b.name
));
}
} else {
s.push_str(&format!(
"{}\n {}",
colors::gray("summary"),
colors::cyan_bold(&baseline.name)
));

for b in benchmarks.iter().filter(|b| *b != baseline) {
let faster = b.stats.avg >= baseline.stats.avg;
let diff = f64::from_str(&format!(
"{:.2}",
1.0 / baseline.stats.avg * b.stats.avg
))
.unwrap();
let inv_diff = f64::from_str(&format!(
"{:.2}",
1.0 / b.stats.avg * baseline.stats.avg
))
.unwrap();
s.push_str(&format!(
"{}\n {}",
colors::gray("summary"),
colors::cyan_bold(&baseline.name)
"\n {}x {} than {}",
if faster {
colors::green(diff.to_string()).to_string()
} else {
colors::red(inv_diff.to_string()).to_string()
},
if faster { "faster" } else { "slower" },
colors::cyan_bold(&b.name)
));

for b in benchmarks.iter().filter(|b| *b != baseline) {
let faster = b.stats.avg >= baseline.stats.avg;
let diff = f64::from_str(&format!(
"{:.2}",
1.0 / baseline.stats.avg * b.stats.avg
))
.unwrap();
let inv_diff = f64::from_str(&format!(
"{:.2}",
1.0 / b.stats.avg * baseline.stats.avg
))
.unwrap();
s.push_str(&format!(
"\n {}x {} than {}",
if faster {
colors::green(diff.to_string()).to_string()
} else {
colors::red(inv_diff.to_string()).to_string()
},
if faster { "faster" } else { "slower" },
colors::cyan_bold(&b.name)
));
}
}

s
Expand Down
9 changes: 3 additions & 6 deletions cli/tools/bench/reporters.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ impl BenchReporter for ConsoleReporter {
let options = self.options.as_mut().unwrap();

options.percentiles = true;
options.colors = colors::use_color();

if FIRST_PLAN
.compare_exchange(true, false, Ordering::SeqCst, Ordering::SeqCst)
Expand Down Expand Up @@ -246,10 +245,9 @@ impl BenchReporter for ConsoleReporter {
}

fn report_group_summary(&mut self) {
let options = match self.options.as_ref() {
None => return,
Some(options) => options,
};
if self.options.is_none() {
return;
}

if 2 <= self.group_measurements.len()
&& (self.group.is_some() || (self.group.is_none() && self.baseline))
Expand All @@ -275,7 +273,6 @@ impl BenchReporter for ConsoleReporter {
},
})
.collect::<Vec<mitata::reporter::GroupBenchmark>>(),
options
)
);
}
Expand Down

0 comments on commit 7d93704

Please sign in to comment.