Document the benchmarks, make them build and compare versions again, and pin Google Benchmark - #5556
Merged
Merged
Conversation
The benchmark project hasn't configured since #4793: download_test_data.cmake compiles cmake/detect_libcpp_version.cpp relative to CMAKE_SOURCE_DIR, which is tests/benchmarks when that is the top-level project, so try_run fails and so does `make run_benchmarks`. The path is now relative to the module itself, which is the same file for the main build. The Dump benchmark discarded dump()'s result, which is [[nodiscard]] by now; it warned, and left the optimizer free to shorten the loop. The result is now kept with benchmark::DoNotOptimize. A new cache variable, JSON_BENCHMARK_INCLUDE_DIR, names the directory holding the nlohmann/json.hpp to benchmark (single_include by default, as before), so the same benchmarks can be built against two versions and compared. tests/benchmarks/README.md documents what is measured, how to build and run the benchmarks, how to read the output, and how to compare two versions with Google Benchmark's compare.py; it recommends doing so by hand before a release rather than in CI. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
5 tasks
The target has configured ${PROJECT_SOURCE_DIR}/benchmarks since it was
added in #2561, but the benchmarks live in tests/benchmarks.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
The benchmarks fetched Google Benchmark's main branch, so two builds on different days could measure with different library code, and CMake 3.30 and later warn that the single-argument FetchContent_Populate() is deprecated. Fetch the 1.9.5 release archive, verified by its SHA-256, with FetchContent_MakeAvailable() instead. That needs CMake 3.14; Google Benchmark itself already needed 3.13. Its -Werror is switched off, so a newer compiler's new warnings cannot break the pinned release, and its install rules are no longer added. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
tests/benchmarks/had no documentation, no way to compare two versions, and, as it turned out, didn't build:cmake/download_test_data.cmakecompilesdetect_libcpp_version.cppfrom${CMAKE_SOURCE_DIR}/cmake/. Whentests/benchmarksis the top-level project, that path doesn't exist, sotry_runfails andmake run_benchmarksstops at configure time.Dumpdiscardeddump()'s result, which is[[nodiscard]]by now. That warns, and it leaves the optimizer free to shorten the measured loop.ci_benchmarksincmake/ci.cmakehas configured${PROJECT_SOURCE_DIR}/benchmarkssince CI targets #2561, but the benchmarks live intests/benchmarks.mainbranch, so builds on different days could measure with different library code. CMake 3.30 and later also warn that the single-argumentFetchContent_Populate()is deprecated.The change
download_test_data.cmaketakes the source from${CMAKE_CURRENT_LIST_DIR}, the directory of the module itself. For the main build that is the same file as before; its configure still detects the standard library.Dumpkeeps the result withbenchmark::DoNotOptimize.ci_benchmarkspoints attests/benchmarks.Google Benchmark is pinned to release 1.9.5. CMake fetches the release archive, checks its SHA-256, and uses
FetchContent_MakeAvailable(), which needs CMake 3.14 (Google Benchmark itself already needed 3.13). Its-Werroris off, so a newer compiler's new warnings can't break the pinned release, and its install rules are no longer added.A cache variable,
JSON_BENCHMARK_INCLUDE_DIR, names the directory holding thenlohmann/json.hppto benchmark. It defaults tosingle_include, as before, so the same benchmarks can be built against two headers.tests/benchmarks/README.mdcovers:compare.py, and how to get stable numbers.It suggests running the comparison by hand before a release and for performance PRs, but not in CI, where shared runners are too noisy.
Verification
--benchmark_filterto keep the run short. Both builds configure (one downloading the test data, one reusing it), build without warnings, and run;compare.pycompares the two result files.developparsestwitter.json47% faster than 3.12.0 (p = 0.0002, 10 repetitions each).FetchContent_Populatedeprecation warning is gone). The benchmarks build with Apple clang and GCC 16, and the compare recipe runs end to end, including v1.9.5's pinnedtools/requirements.txton Python 3.11.JSON_CI=ON, theci_benchmarkstarget's command now namestests/benchmarks.Public API impact
No breaking changes. This touches only the benchmark project, its docs, and the CMake module's path to a helper file. Building the benchmarks now needs CMake 3.14 instead of the nominal 3.11.
make amalgamate. (not applicable)🤖 Generated with Claude Code