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.

Sign up for GitHub

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

Jump to bottom

move input generation for json benchmark to device #10281

Merged
merged 5 commits into from
Feb 26, 2022
Merged

move input generation for json benchmark to device #10281

merged 5 commits into from
Feb 26, 2022

Conversation

karthikeyann
Copy link
Contributor

  • Use device functions to move input generation to device in json benchmark.
  • Use raw string literal for readability
  • replace BENCHMARK_CAPTURE with benchmark fixture without template.

Splitting PR #10109 for review

@karthikeyann karthikeyann added 3 - Ready for Review Ready for review by team tests Unit testing for project libcudf Affects libcudf (C++/CUDA) code. 4 - Needs Review Waiting for reviewer to review or respond Performance Performance related issue non-breaking Non-breaking change labels Feb 14, 2022
@karthikeyann karthikeyann requested a review from a team as a code owner February 14, 2022 15:47
@karthikeyann karthikeyann self-assigned this Feb 14, 2022
@github-actions github-actions bot added the CMake CMake build issue label Feb 14, 2022
@karthikeyann karthikeyann added the feature request New feature or request label Feb 14, 2022
@codecov
Copy link

codecov bot commented Feb 14, 2022 •
edited
Loading

Codecov Report

Merging #10281 (ca06ab0) into branch-22.04 (a7d88cd) will increase coverage by 0.24%.
The diff coverage is n/a.

❗ Current head ca06ab0 differs from pull request most recent head f57698b. Consider uploading reports for the commit f57698b to get more accurate results

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.04   #10281      +/-   ##
================================================
+ Coverage         10.42%   10.67%   +0.24%     
================================================
  Files               119      122       +3     
  Lines             20603    20873     +270     
================================================
+ Hits               2148     2228      +80     
- Misses            18455    18645     +190
Impacted Files Coverage Δ
python/cudf/cudf/_fuzz_testing/fuzzer.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/main.py 0.00% <ø> (ø)
python/cudf/cudf/_version.py 0.00% <ø> (ø)
python/cudf/cudf/comm/gpuarrow.py 0.00% <ø> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/column.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/datetime.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/methods.py 0.00% <ø> (ø)
... and 62 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 463266f...f57698b. Read the comment docs.

nvdbaranec
nvdbaranec approved these changes Feb 24, 2022
View reviewed changes
Copy link
Contributor

@nvdbaranec nvdbaranec left a comment

Choose a reason for hiding this comment

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

Looks good.

@karthikeyann karthikeyann requested review from a team February 24, 2022 19:55
robertmaynard
robertmaynard approved these changes Feb 25, 2022
View reviewed changes
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

Approving the CMake changes

robertmaynard
robertmaynard approved these changes Feb 25, 2022
View reviewed changes
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

Device side data generation LGTM

@karthikeyann
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 3e33453 into rapidsai:branch-22.04 Feb 26, 2022
@karthikeyann karthikeyann mentioned this pull request Mar 7, 2022
Merged
rapids-bot bot pushed a commit that referenced this pull request Mar 22, 2022
@karthikeyann
generate benchmark input in device (#10109)
76c772e 
To speedup generate benchmark input generation, move all data generation to device.
To address #5773 (comment)
This PR moves the random input generation to device.

Rest all of the original work in this PR was split to multiple PRs and merged.
#10277
#10278
#10279
#10280
#10281
#10300

With all of these changes, single iteration of all benchmark runs in <1000 seconds. (from 3067s to 964s).
Running more iterations would see higher benefit too because the benchmark is restarted several times during run which again calls benchmark input generation code.

closes #9857

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Vukasin Milovanovic (https://github.com/vuule)
  - David Wendt (https://github.com/davidwendt)

URL: #10109
@GregoryKimball GregoryKimball mentioned this pull request Jun 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertmaynard robertmaynard robertmaynard approved these changes

@nvdbaranec nvdbaranec nvdbaranec approved these changes

Assignees

@karthikeyann karthikeyann

Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond CMake CMake build issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Performance Performance related issue tests Unit testing for project
Projects
None yet
Milestone
C++ Benchmark Runtime Improvements
Development

Successfully merging this pull request may close these issues.
3 participants
@karthikeyann @robertmaynard @nvdbaranec