Skip to content

Prefer the term metric over stat #1332

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

Merged
merged 2 commits into from
May 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions site/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub mod bootstrap {
}

pub mod comparison {
use crate::comparison::Stat;
use crate::comparison::Metric;
use collector::Bound;
use database::{BenchmarkData, Date};
use serde::{Deserialize, Serialize};
Expand All @@ -140,7 +140,7 @@ pub mod comparison {
pub struct Request {
pub start: Bound,
pub end: Bound,
pub stat: Stat,
pub stat: Metric,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down
30 changes: 15 additions & 15 deletions site/src/comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ pub async fn handle_triage(
let mut before = start.clone();

let mut num_comparisons = 0;
let stat = Stat::Instructions;
let metric = Metric::Instructions;
let benchmark_map = ctxt.get_benchmark_category_map().await;
loop {
let comparison =
match compare_given_commits(before, next.clone(), stat, ctxt, &master_commits)
match compare_given_commits(before, next.clone(), metric, ctxt, &master_commits)
.await
.map_err(|e| format!("error comparing commits: {}", e))?
{
Expand Down Expand Up @@ -86,7 +86,7 @@ pub async fn handle_triage(

// Summarize the entire triage from start commit to end commit
let summary =
match compare_given_commits(start.clone(), end.clone(), stat, ctxt, master_commits)
match compare_given_commits(start.clone(), end.clone(), metric, ctxt, master_commits)
.await
.map_err(|e| format!("error comparing beginning and ending commits: {}", e))?
{
Expand Down Expand Up @@ -181,7 +181,7 @@ async fn populate_report(
}

#[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)]
pub enum Stat {
pub enum Metric {
#[serde(rename = "instructions:u")]
Instructions,
#[serde(rename = "cycles:u")]
Expand All @@ -198,7 +198,7 @@ pub enum Stat {
CpuClock,
}

impl Stat {
impl Metric {
pub fn as_str(&self) -> &'static str {
match self {
Self::Instructions => "instructions:u",
Expand Down Expand Up @@ -586,19 +586,19 @@ pub fn write_summary_table_footer(result: &mut String) {
pub async fn compare(
start: Bound,
end: Bound,
stat: Stat,
metric: Metric,
ctxt: &SiteCtxt,
) -> Result<Option<ArtifactComparison>, BoxedError> {
let master_commits = &ctxt.get_master_commits().commits;

compare_given_commits(start, end, stat, ctxt, master_commits).await
compare_given_commits(start, end, metric, ctxt, master_commits).await
}

/// Compare two bounds on a given stat
/// Compare two bounds on a given metric
async fn compare_given_commits(
start: Bound,
end: Bound,
stat: Stat,
metric: Metric,
ctxt: &SiteCtxt,
master_commits: &[collector::MasterCommit],
) -> Result<Option<ArtifactComparison>, BoxedError> {
Expand All @@ -612,12 +612,12 @@ async fn compare_given_commits(
};
let aids = Arc::new(vec![a.clone(), b.clone()]);

// get all crates, cache, and profile combinations for the given stat
// get all crates, cache, and profile combinations for the given metric
let query = selector::Query::new()
.set::<String>(Tag::Benchmark, selector::Selector::All)
.set::<String>(Tag::Scenario, selector::Selector::All)
.set::<String>(Tag::Profile, selector::Selector::All)
.set(Tag::Metric, selector::Selector::One(stat.as_str()));
.set(Tag::Metric, selector::Selector::One(metric.as_str()));

// `responses` contains series iterators. The first element in the iterator is the data
// for `a` and the second is the data for `b`
Expand All @@ -628,7 +628,7 @@ async fn compare_given_commits(
let statistics_for_b = statistics_from_series(&mut responses);

let mut historical_data =
HistoricalDataMap::calculate(ctxt, a.clone(), master_commits, stat).await?;
HistoricalDataMap::calculate(ctxt, a.clone(), master_commits, metric).await?;
let comparisons = statistics_for_a
.into_iter()
.filter_map(|(test_case, a)| {
Expand Down Expand Up @@ -864,7 +864,7 @@ impl HistoricalDataMap {
ctxt: &SiteCtxt,
from: ArtifactId,
master_commits: &[collector::MasterCommit],
stat: Stat,
metric: Metric,
) -> Result<Self, BoxedError> {
let mut historical_data = HashMap::new();

Expand All @@ -881,12 +881,12 @@ impl HistoricalDataMap {
});
}

// get all crates, cache, and profile combinations for the given stat
// get all crates, cache, and profile combinations for the given metric
let query = selector::Query::new()
.set::<String>(Tag::Benchmark, selector::Selector::All)
.set::<String>(Tag::Scenario, selector::Selector::All)
.set::<String>(Tag::Profile, selector::Selector::All)
.set(Tag::Metric, selector::Selector::One(stat.as_str()));
.set(Tag::Metric, selector::Selector::One(metric.as_str()));

let mut previous_commit_series = ctxt
.statistic_series(query, previous_commits.clone())
Expand Down
34 changes: 17 additions & 17 deletions site/src/github.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::api::github::Issue;
use crate::comparison::{
deserves_attention_icount, write_summary_table, write_summary_table_footer, ArtifactComparison,
ArtifactComparisonSummary, Direction, Stat,
ArtifactComparisonSummary, Direction, Metric,
};
use crate::load::{Config, SiteCtxt, TryCommit};

Expand Down Expand Up @@ -571,7 +571,7 @@ async fn post_comparison_comment(ctxt: &SiteCtxt, commit: QueuedCommit, is_maste
post_comment(&ctxt.config, pr, body).await;
}

fn make_comparison_url(commit: &QueuedCommit, stat: Stat) -> String {
fn make_comparison_url(commit: &QueuedCommit, stat: Metric) -> String {
format!(
"https://perf.rust-lang.org/compare.html?start={}&end={}&stat={}",
commit.parent_sha,
Expand All @@ -580,15 +580,15 @@ fn make_comparison_url(commit: &QueuedCommit, stat: Stat) -> String {
)
}

async fn calculate_stat_comparison(
async fn calculate_metric_comparison(
ctxt: &SiteCtxt,
commit: &QueuedCommit,
stat: Stat,
metric: Metric,
) -> Result<ArtifactComparison, String> {
match crate::comparison::compare(
collector::Bound::Commit(commit.parent_sha.clone()),
collector::Bound::Commit(commit.sha.clone()),
stat,
metric,
ctxt,
)
.await
Expand All @@ -608,10 +608,10 @@ async fn summarize_run(
let mut message = format!(
"Finished benchmarking commit ({sha}): [comparison url]({comparison_url}).\n\n",
sha = commit.sha,
comparison_url = make_comparison_url(&commit, Stat::Instructions)
comparison_url = make_comparison_url(&commit, Metric::Instructions)
);

let inst_comparison = calculate_stat_comparison(ctxt, &commit, Stat::Instructions).await?;
let inst_comparison = calculate_metric_comparison(ctxt, &commit, Metric::Instructions).await?;

let errors = if !inst_comparison.newly_failed_benchmarks.is_empty() {
let benchmarks = inst_comparison
Expand All @@ -629,35 +629,35 @@ async fn summarize_run(
.summarize_by_category(&benchmark_map);

let mut table_written = false;
let stats = vec![
let metrics = vec![
(
"Instruction count",
Stat::Instructions,
Metric::Instructions,
false,
inst_comparison,
),
(
"Max RSS (memory usage)",
Stat::MaxRSS,
Metric::MaxRSS,
true,
calculate_stat_comparison(ctxt, &commit, Stat::MaxRSS).await?,
calculate_metric_comparison(ctxt, &commit, Metric::MaxRSS).await?,
),
(
"Cycles",
Stat::Cycles,
Metric::Cycles,
true,
calculate_stat_comparison(ctxt, &commit, Stat::Cycles).await?,
calculate_metric_comparison(ctxt, &commit, Metric::Cycles).await?,
),
];

for (title, stat, hidden, comparison) in stats {
for (title, metric, hidden, comparison) in metrics {
message.push_str(&format!(
"\n### [{title}]({})\n",
make_comparison_url(&commit, stat)
make_comparison_url(&commit, metric)
));

let (primary, secondary) = comparison.summarize_by_category(&benchmark_map);
table_written |= write_stat_summary(primary, secondary, hidden, &mut message).await;
table_written |= write_metric_summary(primary, secondary, hidden, &mut message).await;
}

if table_written {
Expand All @@ -677,7 +677,7 @@ async fn summarize_run(
}

/// Returns true if a summary table was written to `message`.
async fn write_stat_summary(
async fn write_metric_summary(
primary: ArtifactComparisonSummary,
secondary: ArtifactComparisonSummary,
hidden: bool,
Expand Down