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 log messages about cuIO's nvCOMP and cuFile use #13132

Merged
merged 18 commits into from
Apr 24, 2023

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Apr 13, 2023

Description

Issue #7861

Added the following log messages:

  • Info message about nvCOMP use for a given compression type (once per type).
  • Error message when a file cannot be read/written with cuFile, and fallback is disabled (per file).
  • Info messages about GDS use vs. bounce buffer (per file).

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vuule vuule added cuIO cuIO issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 13, 2023
@vuule vuule self-assigned this Apr 13, 2023
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Apr 13, 2023
@vuule vuule marked this pull request as ready for review April 18, 2023 06:13
@vuule vuule requested a review from a team as a code owner April 18, 2023 06:13
@vuule vuule requested review from karthikeyann and ttnghia April 18, 2023 06:13
Comment on lines +63 to +69
return lhs.lib_major_version == rhs.lib_major_version and
lhs.lib_minor_version == rhs.lib_minor_version and
lhs.lib_patch_version == rhs.lib_patch_version and
lhs.are_all_integrations_enabled == rhs.are_all_integrations_enabled and
lhs.are_stable_integrations_enabled == rhs.are_stable_integrations_enabled and
lhs.compute_capability_major == rhs.compute_capability_major;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that this cover all the members of feature_status_parameters. So the compiler should generate this operator == for you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Humnn, it seems that the default operator==() is available from C++ 20 :|

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it didn't work without the overload unfortunately.

@@ -273,9 +273,18 @@ std::unique_ptr<cufile_input_impl> make_cufile_input(std::string const& filepath
{
if (cufile_integration::is_gds_enabled()) {
try {
return std::make_unique<cufile_input_impl>(filepath);
auto const cufile_in = std::make_unique<cufile_input_impl>(filepath);
CUDF_LOG_INFO("File successfully opened for reading with GDS.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this log? It succeeds, and nothing special here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal of the messages in this PR is to provide some transparency into what method is used for file IO. We have direct use of GDS (this branch), no GDS (host IO + memcpy), and kvikIO. I included messages that make this selection apparent. FWIW, this is not the default branch, so very few users/devs will see this message.
I guess we can derive that lack of messages means that GDS is used, but I would prefer to log this case as INFO (off by default). If we find that this is too verbose, I'll remove some of these.

@vuule vuule requested a review from ttnghia April 18, 2023 21:34
@vuule
Copy link
Contributor Author

vuule commented Apr 24, 2023

/merge

@rapids-bot rapids-bot bot merged commit 310fe9f into rapidsai:branch-23.06 Apr 24, 2023
@vuule vuule deleted the fea-cuio-integration-logging 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants