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

Flush output streams before creating a process to drop caches #10762

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Apr 29, 2022

Small improvement for the try_drop_l3_cache feature in cuIO benchmarks.
Prevents unflushed output from the original process from intermingling with the output from the popen process.

@vuule vuule added tests Unit testing for project cuIO cuIO issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 29, 2022
@vuule vuule requested a review from a team as a code owner April 29, 2022 18:49
@vuule vuule self-assigned this Apr 29, 2022
@vuule vuule requested review from trxcllnt and nvdbaranec April 29, 2022 18:49
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Apr 29, 2022
@vuule
Copy link
Contributor Author

vuule commented Apr 29, 2022

CC @mythrocks, @robertmaynard

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #10762 (4f21707) into branch-22.06 (84f88ce) will increase coverage by 0.02%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           branch-22.06   #10762      +/-   ##
================================================
+ Coverage         86.40%   86.43%   +0.02%     
================================================
  Files               143      143              
  Lines             22444    22444              
================================================
+ Hits              19393    19399       +6     
+ Misses             3051     3045       -6     
Impacted Files Coverage Δ
python/cudf/cudf/comm/gpuarrow.py 79.76% <ø> (ø)
python/cudf/cudf/core/column/string.py 89.21% <ø> (+0.12%) ⬆️
python/cudf/cudf/core/frame.py 93.41% <ø> (ø)
python/cudf/cudf/core/series.py 95.16% <ø> (ø)
python/custreamz/custreamz/kafka.py 29.16% <ø> (ø)
python/cudf/cudf/core/dataframe.py 93.74% <0.00%> (+0.04%) ⬆️
python/cudf/cudf/core/groupby/groupby.py 91.79% <0.00%> (+0.22%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <0.00%> (+0.30%) ⬆️
python/cudf/cudf/core/column/lists.py 92.91% <0.00%> (+0.83%) ⬆️

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 9b8d26f...4f21707. Read the comment docs.

@@ -145,6 +145,8 @@ std::vector<cudf::size_type> segments_in_chunk(int num_segments, int num_chunks,
// Executes the command and returns stderr output
std::string exec_cmd(std::string_view cmd)
{
// Prevent the output from the command from mixing with the original process' output
fflush(nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this std::fflush? Can we use the std:: prefix if so?

https://en.cppreference.com/w/cpp/io/c/fflush

Copy link
Contributor

@bdice bdice Apr 29, 2022

Choose a reason for hiding this comment

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

May also need #include <cstdio> for completeness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it was std::fflush, but now it is :)
Added the include too.

@vuule vuule requested a review from bdice April 29, 2022 22:40
@vuule
Copy link
Contributor Author

vuule commented Apr 29, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit bf10a94 into rapidsai:branch-22.06 Apr 29, 2022
@vuule vuule deleted the fflush-before-popen branch August 10, 2023 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuIO cuIO issue 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.

3 participants