Add gsl::span vs std::span performance benchmark CI (fixes #1167)#1252
Add gsl::span vs std::span performance benchmark CI (fixes #1167)#1252Gafoor2005 wants to merge 17 commits into
Conversation
Add benchmark suite comparing gsl::span vs std::span performance across is_sorted and min_element operations. Includes GitHub Actions workflow for automated testing on GCC 14, Clang 18, and MSVC with results uploaded as artifacts for performance regression analysis.
Ensure both text and JSON benchmark outputs are properly populated in workflow artifacts by running benchmarks separately instead of using pipe redirection, which was leaving the text file empty.
Test PR for PR-based benchmark
|
@microsoft-github-policy-service agree |
carsonRadtke
left a comment
carsonRadtke
left a comment
There was a problem hiding this comment.
Thanks for getting this put together! It'll be great to have the infrastructure necessary to compare GSL types to standard library types. I have briefly reviewed many of the changes, please address my comments and I will perform a more in-depth review once comments have been resolved. I will also assign Copilot as a reviewer, so you get some more feedback; please address those comments too.
This currently is a proof-of-concept of a really cool feature that will help us develop GSL, but I want to make sure the PR is clean and well-understood before running any workflows and merging your changes.
Also, using AI as a tool to write code is great. Please make sure you understand the code that you are contributing. I want to hold all contributions to a high standard and right now this PR needs some work.
There was a problem hiding this comment.
Pull request overview
This PR introduces a CI-run benchmark suite to continuously track performance parity between gsl::span and std::span, generating a per-PR Markdown report and failing CI when gsl::span exceeds a configured slowdown threshold.
Changes:
- Adds a Google Benchmark-based executable comparing
gsl::spanvsstd::spanacross several workloads. - Adds a Python regression checker that parses benchmark JSON, computes
gsl/stdratios, and emits a PR-comment-ready Markdown report. - Adds a GitHub Actions workflow that runs the benchmark across a multi-OS/compiler/standard matrix, uploads results, and posts/updates a single PR comment.
Show a summary per file
| File | Description |
|---|---|
| benchmark/span_bench.cpp | Adds the benchmark program comparing gsl::span and std::span workloads. |
| benchmark/README.md | Documents purpose, local usage, CI matrix, and how regression detection works. |
| benchmark/CMakeLists.txt | Self-contained CMake build that fetches benchmark dependencies and builds against the local GSL checkout. |
| benchmark/check_regression.py | Parses JSON results, pairs Std/GSL benchmarks, computes ratios, and generates Markdown + CI status. |
| .github/workflows/span_benchmark.yml | CI matrix runner that builds/runs benchmarks, aggregates artifacts, and posts/updates a PR comment. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 10
…rkflow - Updated span_benchmark.yml to streamline job configurations and improve artifact handling. - Introduced span_benchmark_comment.yml to automate PR comment updates with benchmark results. - Enhanced CMakeLists.txt to support benchmark builds with new GSL_BENCHMARK option. - Removed README.md from benchmark directory and adjusted check_regression.py for cleaner report generation. - Modified span_bench.cpp to include necessary headers for benchmarking.
| # Strip the trailing _mean / _stddev to get the canonical name | ||
| base_name = name[: name.rfind(f"_{agg}")] |
| constexpr size_t vec_size = 1000; | ||
| v.reserve(vec_size); | ||
| for (int i = 0; i < vec_size; ++i) | ||
| v.push_back(i); |
| for (auto _ : state) | ||
| { | ||
| std::span<const int> sp = vec; | ||
| benchmark::DoNotOptimize(std::min_element(sp.begin(), sp.end())); |
|
I have addressed all your changes and most of the Copilot's too, there are few unresolved Copilot changes which I don't know are that reasonable. I need your opinion on them |
|
I see the updates. I have been out of office the past week and will continue to be out next week. I'll review the changes when I get back the week of the 22nd. Thanks for your patience. |
Summary
Adds a continuous benchmark suite that tracks performance parity between
gsl::spanandstd::spanon every PR, across all supported compilersand platforms.
Closes #1167.
What this adds
benchmark/span_bench.cpp— benchmarks from @galenelias comparinggsl::spanandstd::spanacross five workloads (is_sorted, ranges,custom iterator loop, min_element algorithm, range-for min)
benchmark/CMakeLists.txt— self-contained build; fetchesgoogle-benchmark v1.9.0 and googletest via FetchContent; links against
the local repo's GSL (not a pinned tag) so every PR tests its own
changes
benchmark/check_regression.py— parses benchmark JSON output, pairsStdSpan/GslSpan variants by name, computes
gsl_ns / std_nsratio,writes a Markdown table, exits 1 on regression
.github/workflows/span_benchmark.yml— runs on every PR across 13configs (GCC 13/14, Clang 17/18, MSVC 2022, clang-cl, Apple Clang ×
C++20/23); posts or updates a single PR comment with results from all
configs
Approach
The comparison is in-run ratio (
gsl_ns / std_ns) rather thancomparing against a stored baseline. Both span variants run in the same
process on the same machine at the same moment, so GitHub-hosted runner
noise (~10–15%) cancels out. A ratio above 1.15 (15% slower than
std::span) fails CI.This sidesteps the "Status: Blocked" concern about noisy shared runners —
no self-hosted runner or external service needed.
Each benchmark runs 10 repetitions; the script uses the mean and
reports stddev so reviewers can see measurement stability.
CI matrix
Example PR comment
The workflow posts (and updates) a single comment per PR that looks like:
Notes
-fbounds-safetyconfig is stubbed out in the matrix with a commentexplaining it's not yet stable in mainline Clang releases — easy to
uncomment when it lands
CMakeLists.txthash so CI doesn'tre-clone on every run
peter-evans/find-comment+create-or-update-commentso repeated pushes to the same PR updatethe existing comment rather than flooding the thread