feat: add C benchmark harness to @stdlib/bench - #15400
0PrashantYadav0 wants to merge 3 commits into
Conversation
Signed-off-by: 0PrashantYadav0 <prashantyadav09783@gmail.com>
Signed-off-by: 0PrashantYadav0 <prashantyadav09783@gmail.com>
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: passed
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
| * | ||
| * ## Notes | ||
| * | ||
| * - Benchmark functions always receive `len`. Scalar benchmarks which do not use it should mark it unused to keep compilation clean under `-Wextra`. |
There was a problem hiding this comment.
We should rethink this. It may make sense to just have separate macros for the two benchmark use cases (i.e., scalar and length-based benchmarks). Always passing a len variable does not seem like a good idea, even when benchmark logic doesn't need it.
There was a problem hiding this comment.
Agreed. Split them in d97a61e:
STDLIB_BENCHMARK( fn )→static double fn( int iterations ), run viaSTDLIB_RUN_BENCHMARK( fn )insideSTDLIB_BENCH_PREAMBLE( repeats, iterations )STDLIB_LENGTH_BENCHMARK( fn )→static double fn( int iterations, int len ), run viaSTDLIB_RUN_LENGTH_BENCHMARK( fn )insideSTDLIB_BENCH_LENGTH_PREAMBLE( repeats, iterations, min, max )
There was a problem hiding this comment.
len now only exists in the length-based variants, and STDLIB_BENCHMARK_UNUSED is gone since it only existed to silence the unused len in scalar benchmarks.
There was a problem hiding this comment.
I went with STDLIB_LENGTH_BENCHMARK / STDLIB_RUN_LENGTH_BENCHMARK to mirror STDLIB_BENCH_LENGTH_PREAMBLE. Happy to rename if you'd prefer a different form (e.g., STDLIB_BENCHMARK_LENGTH).
| static void stdlib_bench_main( void ); \ | ||
| static int stdlib_bench_count = 0; \ | ||
| int main( void ) { \ | ||
| srand( time( NULL ) ); \ | ||
| stdlib_bench_print_version(); \ | ||
| stdlib_bench_main(); \ | ||
| stdlib_bench_print_summary( stdlib_bench_count, stdlib_bench_count ); \ | ||
| return 0; \ | ||
| } \ | ||
| static void stdlib_bench_main( void ) |
There was a problem hiding this comment.
Not clear how this is working. Where is stdlib_bench_main actually implemented?
There was a problem hiding this comment.
The macro ends with the signature of stdlib_bench_main and no body, so the { ... } block the author writes after STDLIB_BENCH becomes its body. I.e.,
STDLIB_BENCH {
// ...
}There was a problem hiding this comment.
expands to
static void stdlib_bench_main( void );
static int stdlib_bench_count = 0;
int main( void ) {
srand( time( NULL ) );
stdlib_bench_print_version();
stdlib_bench_main();
stdlib_bench_print_summary( stdlib_bench_count, stdlib_bench_count );
return 0;
}
static void stdlib_bench_main( void ) {
// ...
}There was a problem hiding this comment.
The forward declaration lets the generated main call it before its body appears. That is also why there is no STDLIB_BENCH_EPILOGUE: main prints the summary after the block returns, so it cannot be forgotten. Same idea as STDLIB_NAPI_MODULE_EXPORT_FCN generating stdlib_napi_module_export_fcn_init, except the body is supplied by the caller.
I have spelled this expansion out in the header docs in d97a61e.
If you would rather avoid a generated main, the alternative is STDLIB_BENCH expanding to just int main( void ), with explicit begin/end statements inside the block (e.g., STDLIB_BENCH_PREAMBLE; ... STDLIB_BENCH_EPILOGUE;), at the cost of two lines per file which can be omitted. Let me know which you prefer.
|
A few comments:
|
Signed-off-by: 0PrashantYadav0 <prashantyadav09783@gmail.com>
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: passed
- task: lint_c_benchmarks
status: passed
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
Resolves #13480.
Description
This pull request:
include/stdlib/bench.h) to@stdlib/bench, so C benchmark files no longer hand-roll TAP output, timing, PRNG helpers, and the length/iteration loop. The header depends only on the C standard library and emits the same TAP 13 output as the existing hand-written benchmarks.manifest.json(header-only, no dependencies), a C example, and aC APIssection to the@stdlib/benchREADME documenting every macro and helper.tools/scripts/compile_c_benchmarkto resolve the enclosing package name and pass it tomakeasBENCHMARK_NAME(overridable via the environment). ThenativeC benchmark Makefile snippet turns a non-emptyBENCHMARK_NAMEinto-DSTDLIB_BENCH_NAME="<package name>", so C benchmark names match the package-qualified names already used by JavaScript benchmarks (which readrequire( './../package.json' ).name). This addresses note 7 of the RFC: across files namedbenchmark.cthere are currently 1,327#define NAMElines but only 1,033 distinct values.blas/ext/base/zindex-of-truthy's C benchmark onto the harness as a pilot (200 → 105 lines). Its output name changes from# c::zindex_of_truthy:len=10to# c::@stdlib/blas/ext/base/zindex-of-truthy:len=10.Existing C benchmarks are untouched: their Makefiles ignore the extra
BENCHMARK_NAMEvariable, and in Makefiles which include the new block an emptyBENCHMARK_NAMEyields no-Dflag.Example
Differences from the RFC sketch
NAMEisSTDLIB_BENCH_NAME, so the build can define it without colliding with the unconditional#define NAMEin existing benchmark files.STDLIB_BENCH_EPILOGUEis not needed:STDLIB_BENCHgeneratesmain()around a wrapper function, so the TAP summary is printed automatically.STDLIB_BENCHMARK_FILL_STRIDED_ARRAY_FLOAT64isSTDLIB_BENCHMARK_FILL_ARRAY(the sketch's signature had no stride, and assignment is type-agnostic).STDLIB_BENCH_PREAMBLE( repeats, iterations )for scalar benchmarks,STDLIB_BENCH_PRINT_NAME()/STDLIB_BENCH_PRINT_NAME_F( fmt, ... )for the# c::line (note 6 of the RFC), andSTDLIB_BENCHMARK_UNUSED( x )so scalar benchmarks compile cleanly under-Wextra.-Iand nothing is linked or compiled.Verification
make examples-c EXAMPLES_FILTER=".*/@stdlib/bench/examples/c/.*"— passes.make benchmark-c BENCHMARKS_FILTER=".*/blas/ext/base/zindex-of-truthy/benchmark/c/.*"— 36 benchmarks,1..36,# ok, no compiler warnings (also clean under-Wextra).BENCHMARK_NAME=custom make benchmark-c ...— override respected; compiling without the define falls back to the in-file name.make benchmark-con an unmigrated benchmark (math/base/special/abs) — unchanged behavior.make lint-markdown-files FILES="lib/node_modules/@stdlib/bench/README.md"— passes.Related Issues
This pull request has the following related issues:
Questions
STDLIB_RUN_BENCHMARKkeeps the name from the RFC sketch, which does not follow theSTDLIB_BENCH_/STDLIB_BENCHMARK_prefix used by every other macro. It is documented as the one exception; happy to rename it toSTDLIB_BENCHMARK_RUNif preferred.tools/snippets/benchmark/c/benchmark.cis intentionally left unchanged so that adopting the harness for newly scaffolded packages can be decided separately.Other
shellcheckwithout a severity filter) reportsSC2153/SC2181ontools/scripts/compile_c_benchmark. Nineteen of the twenty-one findings pre-exist this change; the two new ones follow the file's existing pattern (name="${NAME}"andif [[ "$?" -ne 0 ]]).Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
AI assistant reviewed the design decisions and the resulting changes, and the verification steps listed above were run locally.
@stdlib-js/reviewers