golang benchmarks can't handle multiple metrics #118
Labels
enhancement
New feature or request
Comments
|
Closed as resolved in this PR: #177 |
ningziwen
added a commit
to runfinch/finch
that referenced
this issue
Jul 7, 2023
*Description of changes:* The Go multiple metric benchmarking was fixed in upstream. benchmark-action/github-action-benchmark#118 Backfilling it for Finch. Backfilled by the script ``` let obj = window.BENCHMARK_DATA; for (let benchmark of obj.entries["Finch Benchmark"]) { let newBenches = []; for (let bench of benchmark.benches) { let metrics = bench.unit.split("\t").filter(el => el !== ""); // If data already split, skip if(metrics.length === 1) { newBenches.push(bench); continue; } // handle the first metric let firstMetric = metrics[0].trim(); let newBench = JSON.parse(JSON.stringify(bench)); // Deep clone newBench.name += " - " + firstMetric; newBench.unit = firstMetric; newBenches.push(newBench); // handle the rest of the metrics for (let i = 1; i < metrics.length; i++) { let metric = metrics[i].trim().split(" "); let value = metric[0]; let unit = metric[1]; let newBench = JSON.parse(JSON.stringify(bench)); // Deep clone newBench.name += " - " + unit; newBench.unit = unit; newBench.value = value; newBenches.push(newBench); } } benchmark.benches = newBenches; } console.log(JSON.stringify(obj, null, 2)); ``` *Testing done:* By locally exploring the index.html in Chrome. - [ X ] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Ziwen Ning <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
the
extractGoResultfunction works great as long as there's only a single metric on the line.However golang's benchmark tool supports including additional compiler and user defined metrics, which will appear on the same line.
The format is defined semi-formally in a proposal. Key description includes:
I've hacked a potential solution in this fork , happy to PR it into this repo or work on a different approach -- one issue I ran into is that there's some tests that fundamentally assert you're not adding new metrics to an existing test. Additionally there's been some changes to how the go toolchain wants to be invoked since the standardization of gomodules.
Based on that another approach could be making a separate "go 1.16+" test that includes multiple metrics and leave the existing go 1.10 as-is, but that comes with its own drawbacks.
Anyway if there's interest i'm happy to help by cleaning up what i've started in whatever way seems best to the maintainers.
thanks!
The text was updated successfully, but these errors were encountered: