Skip to content

Commit 61194f6

Browse files
committed
fix(prof): openvm-prof handling of duplicate names (#1617)
1 parent bb90132 commit 61194f6

File tree

4 files changed

+38
-8
lines changed

4 files changed

+38
-8
lines changed

.github/workflows/benchmark-call.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,9 @@ jobs:
182182
- name: Set working directory
183183
id: set-working-dir
184184
run: |
185-
WORKING_DIR=$(jq -r --arg name "${{ inputs.benchmark_name }}" '
185+
WORKING_DIR=$(jq -r --arg id "${{ inputs.benchmark_id }}" '
186186
.benchmarks[] |
187-
select(.name == $name) |
187+
select(.id == $id) |
188188
.working_directory
189189
' ./ci/benchmark-config.json)
190190
RELATIVE_PATH=$(python3 -c "import os.path; print(os.path.relpath('.', '$WORKING_DIR'))")

.github/workflows/benchmarks.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ jobs:
199199
- name: Download all metric json files from S3
200200
run: |
201201
matrix=$(echo '${{ needs.create-matrix.outputs.matrix }}')
202+
names=$(echo $matrix | jq -r '.[] | "\(.id)"')
203+
names_list=$(echo -n "$names" | paste -sd "," -)
204+
202205
json_files=$(echo $matrix | jq -r '
203206
.[] |
204207
"\(.id)-${{ env.CURRENT_SHA }}.json"')
@@ -224,6 +227,7 @@ jobs:
224227
225228
openvm-prof --json-paths "${json_file_list}" \
226229
--prev-json-paths "${prev_json_file_list}" \
230+
--names "${names_list}" \
227231
summary \
228232
--benchmark-results-link "https://github.com/${{ github.repository }}/blob/benchmark-results/${BENCHMARK_RESULTS_PATH}" \
229233
--summary-md-path summary.md

crates/prof/src/main.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ struct Cli {
2929
#[arg(long, value_delimiter = ',')]
3030
prev_json_paths: Option<Vec<PathBuf>>,
3131

32+
/// Display names for each metrics file (optional).
33+
/// If provided, must be same length and in same order as `json-paths`.
34+
/// Otherwise, the app program name will be used.
35+
#[arg(long, value_delimiter = ',')]
36+
names: Option<Vec<String>>,
37+
3238
/// Path to write the output JSON in BMF format
3339
#[arg(long)]
3440
output_json: Option<PathBuf>,
@@ -55,10 +61,21 @@ fn main() -> Result<()> {
5561
} else {
5662
vec![None; args.json_paths.len()]
5763
};
64+
let names = args.names.unwrap_or_default();
65+
let mut names = if names.len() == args.json_paths.len() {
66+
names
67+
} else {
68+
vec!["".to_string(); args.json_paths.len()]
69+
};
5870
let mut aggregated_metrics = Vec::new();
5971
let mut md_paths = Vec::new();
6072
let mut output = BenchmarkOutput::default();
61-
for (metrics_path, prev_metrics_path) in args.json_paths.into_iter().zip_eq(prev_json_paths) {
73+
for ((metrics_path, prev_metrics_path), name) in args
74+
.json_paths
75+
.into_iter()
76+
.zip_eq(prev_json_paths)
77+
.zip_eq(&mut names)
78+
{
6279
let db = MetricDb::new(&metrics_path)?;
6380
let grouped = GroupedMetrics::new(&db, "group")?;
6481
let mut aggregated = grouped.aggregate();
@@ -71,8 +88,10 @@ fn main() -> Result<()> {
7188
aggregated.set_diff(prev_aggregated.as_ref().unwrap());
7289
}
7390
}
74-
75-
output.insert(&aggregated.name(), aggregated.to_bencher_metrics());
91+
if name.is_empty() {
92+
*name = aggregated.name();
93+
}
94+
output.insert(name, aggregated.to_bencher_metrics());
7695
let mut writer = Vec::new();
7796
aggregated.write_markdown(&mut writer, VM_METRIC_NAMES)?;
7897

@@ -95,8 +114,12 @@ fn main() -> Result<()> {
95114
if let Some(command) = args.command {
96115
match command {
97116
Commands::Summary(cmd) => {
98-
let summary =
99-
GithubSummary::new(&aggregated_metrics, &md_paths, &cmd.benchmark_results_link);
117+
let summary = GithubSummary::new(
118+
&names,
119+
&aggregated_metrics,
120+
&md_paths,
121+
&cmd.benchmark_results_link,
122+
);
100123
let mut writer = Vec::new();
101124
summary.write_markdown(&mut writer)?;
102125
if let Some(path) = cmd.summary_md_path {

crates/prof/src/summary.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,16 @@ pub struct SingleSummaryMetrics {
4242

4343
impl GithubSummary {
4444
pub fn new(
45+
names: &[String],
4546
aggregated_metrics: &[(AggregateMetrics, Option<AggregateMetrics>)],
4647
md_paths: &[PathBuf],
4748
benchmark_results_link: &str,
4849
) -> Self {
4950
let rows = aggregated_metrics
5051
.iter()
5152
.zip_eq(md_paths.iter())
52-
.map(|((aggregated, prev_aggregated), md_path)| {
53+
.zip_eq(names)
54+
.map(|(((aggregated, prev_aggregated), md_path), name)| {
5355
let md_filename = md_path.file_name().unwrap().to_str().unwrap();
5456
let mut row = aggregated.get_summary_row(md_filename).unwrap();
5557
if let Some(prev_aggregated) = prev_aggregated {
@@ -60,6 +62,7 @@ impl GithubSummary {
6062
}
6163
}
6264
}
65+
row.name = name.clone();
6366
row
6467
})
6568
.collect();

0 commit comments

Comments
 (0)