Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add templated benchmark with fixture #9838

Merged

Conversation

karthikeyann
Copy link
Contributor

@karthikeyann karthikeyann commented Dec 3, 2021

BENCHMARK_TEMPLATE_F is for non-templated benchmark with templated fixture.
BENCHMARK_F is for non-templated benchmark with non-templated fixture.
Google benchmark does not have support for templated benchmark function with non-templated fixture.

Often, BENCHMARK_TEMPLATE_F is used as a proxy for templated benchmark. But templated fixture is not really required here. It also has limitation of specifying different name for each template.
So, this PR extends google benchmark to support templated benchmark with non-templated fixture.

  • Use TEMPLATED_BENCHMARK_F in compiled binary op.
  • Use in other relevant benchmarks as well.

Usage:
TEMPLATED_BENCHMARK_F(FixtureClass, TemplateFunction, ...);
... are template arguments

Example:

class FixtureClass : public cudf::benchmark {
};

template<typename T, typename U>
void TemplateFunction(benchmark::State& state) {
     for (auto _ : state) {
       // benchmark stuff
     }
}
TEMPLATED_BENCHMARK_F(FixtureClass, TemplateFunction, int, double)->Range(128, 512);

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Dec 3, 2021
@karthikeyann karthikeyann added 2 - In Progress Currently a work in progress 4 - Needs Review Waiting for reviewer to review or respond improvement Improvement / enhancement to an existing function non-breaking Non-breaking change tests Unit testing for project and removed libcudf Affects libcudf (C++/CUDA) code. labels Dec 3, 2021
@karthikeyann karthikeyann self-assigned this Dec 3, 2021
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Dec 3, 2021
@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

Merging #9838 (791097c) into branch-22.02 (967a333) will decrease coverage by 0.04%.
The diff coverage is 5.79%.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.02    #9838      +/-   ##
================================================
- Coverage         10.49%   10.44%   -0.05%     
================================================
  Files               119      119              
  Lines             20305    20422     +117     
================================================
+ Hits               2130     2133       +3     
- Misses            18175    18289     +114     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <ø> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/groupby/groupby.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <ø> (ø)
python/cudf/cudf/core/indexed_frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/multiindex.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/json.py 0.00% <ø> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50e22ab...791097c. Read the comment docs.

@karthikeyann karthikeyann marked this pull request as ready for review December 7, 2021 08:32
@karthikeyann karthikeyann requested a review from a team as a code owner December 7, 2021 08:32
@karthikeyann karthikeyann added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Dec 7, 2021
@karthikeyann karthikeyann requested a review from harrism December 7, 2021 08:35
Copy link
Contributor

@devavret devavret left a comment

Choose a reason for hiding this comment

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

cool stuff

@robertmaynard
Copy link
Contributor

robertmaynard commented Dec 7, 2021

I wonder if we should just switch these tests over to NVBench as IIRC it already supports this feature.

edit: https://github.com/NVIDIA/nvbench/blob/ff507596bfa76a3875dffade60cb8747cd8f41fc/docs/benchmarks.md#type-axes

@karthikeyann
Copy link
Contributor Author

karthikeyann commented Dec 7, 2021

NVIDIA/nvbench#24 Found this issue. It doesn't have support for fixtures yet.
RAII is good enough.

We should move to nvbench #7960 eventually.
#7960 (comment) compare.py equivalent is still pending.

@karthikeyann
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit e6b0661 into rapidsai:branch-22.02 Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change tests Unit testing for project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants