Skip to content

feat: implement baseline ratio computation and unit normalization in benchmark reporting#748

Merged
vbreuss merged 3 commits intomainfrom
topic/calculate-ratio-for-baseline
May 1, 2026
Merged

feat: implement baseline ratio computation and unit normalization in benchmark reporting#748
vbreuss merged 3 commits intomainfrom
topic/calculate-ratio-for-baseline

Conversation

@vbreuss
Copy link
Copy Markdown
Member

@vbreuss vbreuss commented May 1, 2026

This pull request enhances the BenchmarkReport functionality to improve the handling and comparison of benchmark data, especially when working with baselines that may use different units for time and memory. The main changes include normalizing units for accurate ratio calculations, recomputing ratios for baseline rows, and ensuring empty rows are formatted consistently with the table header. Comprehensive tests have been added to verify these behaviors.

Enhancements to Baseline Handling and Ratio Computation:

  • Added logic to normalize time and memory units (e.g., ns, μs, ms, KB, MB) to a common base (nanoseconds for time, bytes for memory) for accurate ratio calculations between baseline and current benchmark results.
  • Implemented the RecomputeBaselineRatio method to recompute the "Ratio" and "Alloc Ratio" values for baseline rows relative to the current benchmark, ensuring unit differences are handled and results are accurate.

Table Formatting Improvements:

  • Modified the table rendering logic to emit empty rows that match the header's column count when separating parameter groups, improving the readability and consistency of the generated tables.

Utility and Helper Improvements:

  • Introduced FindColumnIndex for case-insensitive column name lookup, improving robustness when handling table headers.

Testing Enhancements:

  • Added and extended tests to verify unit normalization, baseline ratio recomputation, empty row formatting, and column index lookup, ensuring reliability and correctness of new features.

@vbreuss vbreuss self-assigned this May 1, 2026
Copilot AI review requested due to automatic review settings May 1, 2026 15:48
@vbreuss vbreuss added the enhancement New feature or request label May 1, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements baseline ratio recomputation in benchmark PR comment rendering by normalizing time/memory units before computing ratios, ensuring baseline comparisons remain accurate across unit changes.

Changes:

  • Recompute injected baseline Ratio and Alloc Ratio values relative to the current Mockolate row.
  • Add quantity parsing/normalization to compare values across time (ps/ns/μs/ms/s) and memory (B/KB/MB/GB) units.
  • Extend Build.Tests coverage for column lookup, ratio recomputation, and unit normalization behaviors.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
Pipeline/BenchmarkReport.cs Adds unit-normalized quantity parsing and recomputes baseline ratios when injecting baseline rows.
Tests/Build.Tests/BuildBodyTests.cs Adds end-to-end tests validating recomputed ratios and unit normalization in rendered output.
Tests/Build.Tests/BenchmarkReportHelpersTests.cs Adds focused helper-method tests for column lookup, ratio recomputation, and quantity parsing.

@vbreuss vbreuss force-pushed the topic/calculate-ratio-for-baseline branch from 7a81f44 to 1894670 Compare May 1, 2026 15:54
@vbreuss vbreuss force-pushed the topic/calculate-ratio-for-baseline branch from 1894670 to 991de57 Compare May 1, 2026 15:55
Copilot AI review requested due to automatic review settings May 1, 2026 15:55
@vbreuss vbreuss force-pushed the topic/calculate-ratio-for-baseline branch from 991de57 to 36f565e Compare May 1, 2026 15:55
@vbreuss vbreuss enabled auto-merge (squash) May 1, 2026 15:56
@vbreuss vbreuss disabled auto-merge May 1, 2026 15:56
@vbreuss vbreuss merged commit 29efc63 into main May 1, 2026
1 of 13 checks passed
@vbreuss vbreuss deleted the topic/calculate-ratio-for-baseline branch May 1, 2026 15:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@@ -11,8 +12,6 @@ namespace Build;
/// </summary>
internal static class BenchmarkReport
{
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultColumnsToRemove was removed from BenchmarkReport, but it is still referenced by the build pipeline (Pipeline/Build.Benchmarks.cs calls BenchmarkReport.DefaultColumnsToRemove). This will break compilation for the Pipeline project. Either restore this constant/field, or update the call site to pass the column list another way (e.g., a local constant or configuration).

Suggested change
{
{
public static readonly string[] DefaultColumnsToRemove = Array.Empty<string>();

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@vbreuss vbreuss mentioned this pull request May 1, 2026
vbreuss added a commit that referenced this pull request May 1, 2026
This pull requests fixes a build error accidentally introduced in #748 that caused the build pipeline to not compile.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2026

This is addressed in release v3.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request state: released The issue is released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants