From 2ff50ef335e5ffa12d3318d3cbdc9f9118875157 Mon Sep 17 00:00:00 2001 From: vuule Date: Wed, 12 Apr 2023 10:00:20 -0700 Subject: [PATCH 1/8] GDS logging --- cpp/src/io/utilities/file_io_utilities.cpp | 32 ++++++++++++++++++---- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/cpp/src/io/utilities/file_io_utilities.cpp b/cpp/src/io/utilities/file_io_utilities.cpp index f1fb50f5340..d96d9248230 100644 --- a/cpp/src/io/utilities/file_io_utilities.cpp +++ b/cpp/src/io/utilities/file_io_utilities.cpp @@ -68,7 +68,7 @@ class cufile_shim { auto is_valid() const noexcept { return init_error == nullptr; } public: - cufile_shim(cufile_shim const&) = delete; + cufile_shim(cufile_shim const&) = delete; cufile_shim& operator=(cufile_shim const&) = delete; static cufile_shim const* instance(); @@ -273,9 +273,20 @@ std::unique_ptr make_cufile_input(std::string const& filepath { if (cufile_integration::is_gds_enabled()) { try { - return std::make_unique(filepath); + auto const cufile_in = std::make_unique(filepath); + CUDF_LOG_INFO("File {} successfully opened for reading with GDS.", filepath); } catch (...) { - if (cufile_integration::is_always_enabled()) throw; + if (cufile_integration::is_always_enabled()) { + CUDF_LOG_ERROR( + "Failed to open file {} for reading with GDS. Enable bounce buffer fallback to read this " + "file.", + filepath); + throw; + } + CUDF_LOG_INFO( + "Failed to open file {} for reading with GDS. Data will be read from the file using a " + "bounce buffer (possible performance impact).", + filepath); } } return nullptr; @@ -285,9 +296,20 @@ std::unique_ptr make_cufile_output(std::string const& filepa { if (cufile_integration::is_gds_enabled()) { try { - return std::make_unique(filepath); + auto const cufile_out = std::make_unique(filepath); + CUDF_LOG_INFO("File {} successfully opened for writing with GDS.", filepath); } catch (...) { - if (cufile_integration::is_always_enabled()) throw; + if (cufile_integration::is_always_enabled()) { + CUDF_LOG_ERROR( + "Failed to open file {} for writing with GDS. Enable bounce buffer fallback to write to " + "this file.", + filepath); + throw; + } + CUDF_LOG_INFO( + "Failed to open file {} for writing with GDS. Data will be written to the file using a " + "bounce buffer (possible performance impact).", + filepath); } } return nullptr; From f2976453556b1cc1678e8a4626a82954c595c11d Mon Sep 17 00:00:00 2001 From: vuule Date: Wed, 12 Apr 2023 17:27:59 -0700 Subject: [PATCH 2/8] nvCOMP logs --- cpp/src/io/comp/nvcomp_adapter.cpp | 79 ++++++++++++++++++++++++++++-- cpp/src/io/comp/nvcomp_adapter.hpp | 13 +++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/cpp/src/io/comp/nvcomp_adapter.cpp b/cpp/src/io/comp/nvcomp_adapter.cpp index 363ae6af1ad..0b34c2f4766 100644 --- a/cpp/src/io/comp/nvcomp_adapter.cpp +++ b/cpp/src/io/comp/nvcomp_adapter.cpp @@ -125,6 +125,16 @@ auto batched_decompress_async(compression_type compression, Args&&... args) } } +std::string compression_type_name(compression_type compression) +{ + switch (compression) { + case compression_type::SNAPPY: return "Snappy"; + case compression_type::ZSTD: return "Zstandard"; + case compression_type::DEFLATE: return "Deflate"; + } + return "compression_type(" + std::to_string(int(compression)) + ")"; +} + size_t batched_decompress_temp_size(compression_type compression, size_t num_chunks, size_t max_uncomp_chunk_size, @@ -439,8 +449,21 @@ feature_status_parameters::feature_status_parameters() cudaDeviceGetAttribute(&compute_capability_major, cudaDevAttrComputeCapabilityMajor, device)); } -std::optional is_compression_disabled(compression_type compression, - feature_status_parameters params) +using feature_status_inputs = std::pair; +struct hash_feature_status_inputs { + size_t operator()(feature_status_inputs const& fsi) const + { + // Outside of unit tests, the same `feature_status_parameters` value will always be passed + // within a run; for simplicity, only use `compression_type` for the hash + return std::hash{}(fsi.first); + } +}; + +using feature_status_memo_map = + std::unordered_map, hash_feature_status_inputs>; + +std::optional is_compression_disabled_impl(compression_type compression, + feature_status_parameters params) { switch (compression) { case compression_type::DEFLATE: { @@ -477,6 +500,30 @@ std::optional is_compression_disabled(compression_type compression, return "Unsupported compression type"; } +std::optional is_compression_disabled(compression_type compression, + feature_status_parameters params) +{ + static feature_status_memo_map comp_status_reason; + if (auto mem_res_it = comp_status_reason.find(feature_status_inputs{compression, params}); + mem_res_it != comp_status_reason.end()) { + return mem_res_it->second; + } + + // The rest of the function will execute only once per run, the memoized result will be returned + // in all subsequent calls with the same compression type + auto const reason = is_compression_disabled_impl(compression, params); + comp_status_reason[{compression, params}] = reason; + if (reason.has_value()) { + CUDF_LOG_INFO("nvCOMP is disabled for {} compression; reason: {}", + compression_type_name(compression), + reason.value()); + } else { + CUDF_LOG_INFO("nvCOMP is enabled for {} compression", compression_type_name(compression)); + } + + return reason; +} + std::optional is_zstd_decomp_disabled(feature_status_parameters const& params) { if (not NVCOMP_HAS_ZSTD_DECOMP( @@ -503,8 +550,8 @@ std::optional is_zstd_decomp_disabled(feature_status_parameters con return std::nullopt; } -std::optional is_decompression_disabled(compression_type compression, - feature_status_parameters params) +std::optional is_decompression_disabled_impl(compression_type compression, + feature_status_parameters params) { switch (compression) { case compression_type::DEFLATE: { @@ -531,6 +578,30 @@ std::optional is_decompression_disabled(compression_type compressio return "Unsupported compression type"; } +std::optional is_decompression_disabled(compression_type compression, + feature_status_parameters params) +{ + static feature_status_memo_map decomp_status_reason; + if (auto mem_res_it = decomp_status_reason.find(feature_status_inputs{compression, params}); + mem_res_it != decomp_status_reason.end()) { + return mem_res_it->second; + } + + // The rest of the function will execute only once per run, the memoized result will be returned + // in all subsequent calls with the same compression type + auto const reason = is_decompression_disabled_impl(compression, params); + decomp_status_reason[{compression, params}] = reason; + if (reason.has_value()) { + CUDF_LOG_INFO("nvCOMP is disabled for {} decompression; reason: {}", + compression_type_name(compression), + reason.value()); + } else { + CUDF_LOG_INFO("nvCOMP is enabled for {} decompression", compression_type_name(compression)); + } + + return reason; +} + size_t compress_input_alignment_bits(compression_type compression) { switch (compression) { diff --git a/cpp/src/io/comp/nvcomp_adapter.hpp b/cpp/src/io/comp/nvcomp_adapter.hpp index a6bde7957c7..ea5a810272a 100644 --- a/cpp/src/io/comp/nvcomp_adapter.hpp +++ b/cpp/src/io/comp/nvcomp_adapter.hpp @@ -55,6 +55,19 @@ struct feature_status_parameters { } }; +/** + * @brief Equality operator overload. Required to use `feature_status_parameters` as a map key. + */ +inline bool operator==(const feature_status_parameters& lhs, const feature_status_parameters& rhs) +{ + 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; +} + /** * @brief If a compression type is disabled through nvCOMP, returns the reason as a string. * From 6dac89ca3024ca1cde6242bdab5628a0389bf5df Mon Sep 17 00:00:00 2001 From: vuule Date: Wed, 12 Apr 2023 18:55:03 -0700 Subject: [PATCH 3/8] remove filepath --- cpp/src/io/utilities/file_io_utilities.cpp | 24 +++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/cpp/src/io/utilities/file_io_utilities.cpp b/cpp/src/io/utilities/file_io_utilities.cpp index d96d9248230..6f4f481de36 100644 --- a/cpp/src/io/utilities/file_io_utilities.cpp +++ b/cpp/src/io/utilities/file_io_utilities.cpp @@ -274,19 +274,17 @@ std::unique_ptr make_cufile_input(std::string const& filepath if (cufile_integration::is_gds_enabled()) { try { auto const cufile_in = std::make_unique(filepath); - CUDF_LOG_INFO("File {} successfully opened for reading with GDS.", filepath); + CUDF_LOG_INFO("File successfully opened for reading with GDS."); } catch (...) { if (cufile_integration::is_always_enabled()) { CUDF_LOG_ERROR( - "Failed to open file {} for reading with GDS. Enable bounce buffer fallback to read this " - "file.", - filepath); + "Failed to open file for reading with GDS. Enable bounce buffer fallback to read this " + "file."); throw; } CUDF_LOG_INFO( - "Failed to open file {} for reading with GDS. Data will be read from the file using a " - "bounce buffer (possible performance impact).", - filepath); + "Failed to open file for reading with GDS. Data will be read from the file using a bounce " + "buffer (possible performance impact)."); } } return nullptr; @@ -297,19 +295,17 @@ std::unique_ptr make_cufile_output(std::string const& filepa if (cufile_integration::is_gds_enabled()) { try { auto const cufile_out = std::make_unique(filepath); - CUDF_LOG_INFO("File {} successfully opened for writing with GDS.", filepath); + CUDF_LOG_INFO("File successfully opened for writing with GDS."); } catch (...) { if (cufile_integration::is_always_enabled()) { CUDF_LOG_ERROR( - "Failed to open file {} for writing with GDS. Enable bounce buffer fallback to write to " - "this file.", - filepath); + "Failed to open file for writing with GDS. Enable bounce buffer fallback to write to " + "this file."); throw; } CUDF_LOG_INFO( - "Failed to open file {} for writing with GDS. Data will be written to the file using a " - "bounce buffer (possible performance impact).", - filepath); + "Failed to open file for writing with GDS. Data will be written to the file using a bounce " + "buffer (possible performance impact)."); } } return nullptr; From 2864b4bab2d634385e2c71dabef26162dd64a0aa Mon Sep 17 00:00:00 2001 From: vuule Date: Thu, 13 Apr 2023 12:20:22 -0700 Subject: [PATCH 4/8] style --- cpp/src/io/comp/nvcomp_adapter.hpp | 2 +- cpp/src/io/utilities/file_io_utilities.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/io/comp/nvcomp_adapter.hpp b/cpp/src/io/comp/nvcomp_adapter.hpp index ea5a810272a..9e4e6b68759 100644 --- a/cpp/src/io/comp/nvcomp_adapter.hpp +++ b/cpp/src/io/comp/nvcomp_adapter.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, NVIDIA CORPORATION. + * Copyright (c) 2022-2023, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/cpp/src/io/utilities/file_io_utilities.cpp b/cpp/src/io/utilities/file_io_utilities.cpp index 6f4f481de36..ebb235802ce 100644 --- a/cpp/src/io/utilities/file_io_utilities.cpp +++ b/cpp/src/io/utilities/file_io_utilities.cpp @@ -68,7 +68,7 @@ class cufile_shim { auto is_valid() const noexcept { return init_error == nullptr; } public: - cufile_shim(cufile_shim const&) = delete; + cufile_shim(cufile_shim const&) = delete; cufile_shim& operator=(cufile_shim const&) = delete; static cufile_shim const* instance(); From 1778d4f8cb9a7d1f57f9d837d2b21c31684758c1 Mon Sep 17 00:00:00 2001 From: Vukasin Milovanovic Date: Mon, 17 Apr 2023 23:03:58 -0700 Subject: [PATCH 5/8] style --- cpp/src/io/utilities/file_io_utilities.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/io/utilities/file_io_utilities.cpp b/cpp/src/io/utilities/file_io_utilities.cpp index ebb235802ce..6f4f481de36 100644 --- a/cpp/src/io/utilities/file_io_utilities.cpp +++ b/cpp/src/io/utilities/file_io_utilities.cpp @@ -68,7 +68,7 @@ class cufile_shim { auto is_valid() const noexcept { return init_error == nullptr; } public: - cufile_shim(cufile_shim const&) = delete; + cufile_shim(cufile_shim const&) = delete; cufile_shim& operator=(cufile_shim const&) = delete; static cufile_shim const* instance(); From a53f3c85f68b364f8f4827fd559161f47f50635c Mon Sep 17 00:00:00 2001 From: Vukasin Milovanovic Date: Tue, 18 Apr 2023 14:20:14 -0700 Subject: [PATCH 6/8] replace C-style cast Co-authored-by: Nghia Truong <7416935+ttnghia@users.noreply.github.com> --- cpp/src/io/comp/nvcomp_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/io/comp/nvcomp_adapter.cpp b/cpp/src/io/comp/nvcomp_adapter.cpp index 0b34c2f4766..f40adc245e4 100644 --- a/cpp/src/io/comp/nvcomp_adapter.cpp +++ b/cpp/src/io/comp/nvcomp_adapter.cpp @@ -132,7 +132,7 @@ std::string compression_type_name(compression_type compression) case compression_type::ZSTD: return "Zstandard"; case compression_type::DEFLATE: return "Deflate"; } - return "compression_type(" + std::to_string(int(compression)) + ")"; + return "compression_type(" + std::to_string(static_cast(compression)) + ")"; } size_t batched_decompress_temp_size(compression_type compression, From a6549ef96513dfcfd3c6e69ec5135d40f9cd7635 Mon Sep 17 00:00:00 2001 From: vuule Date: Tue, 18 Apr 2023 14:20:22 -0700 Subject: [PATCH 7/8] review changes --- cpp/src/io/comp/nvcomp_adapter.cpp | 2 ++ cpp/src/io/comp/nvcomp_adapter.hpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/io/comp/nvcomp_adapter.cpp b/cpp/src/io/comp/nvcomp_adapter.cpp index 0b34c2f4766..a8f71c5865c 100644 --- a/cpp/src/io/comp/nvcomp_adapter.cpp +++ b/cpp/src/io/comp/nvcomp_adapter.cpp @@ -449,6 +449,7 @@ feature_status_parameters::feature_status_parameters() cudaDeviceGetAttribute(&compute_capability_major, cudaDevAttrComputeCapabilityMajor, device)); } +// Represents all parameters reqired to determine status of a compression/decompression feature using feature_status_inputs = std::pair; struct hash_feature_status_inputs { size_t operator()(feature_status_inputs const& fsi) const @@ -459,6 +460,7 @@ struct hash_feature_status_inputs { } }; +// Hash map type that stores feature status for different combinations of input parameters using feature_status_memo_map = std::unordered_map, hash_feature_status_inputs>; diff --git a/cpp/src/io/comp/nvcomp_adapter.hpp b/cpp/src/io/comp/nvcomp_adapter.hpp index 9e4e6b68759..1393b70f058 100644 --- a/cpp/src/io/comp/nvcomp_adapter.hpp +++ b/cpp/src/io/comp/nvcomp_adapter.hpp @@ -58,7 +58,7 @@ struct feature_status_parameters { /** * @brief Equality operator overload. Required to use `feature_status_parameters` as a map key. */ -inline bool operator==(const feature_status_parameters& lhs, const feature_status_parameters& rhs) +inline bool operator==(feature_status_parameters const& lhs, feature_status_parameters const& rhs) { return lhs.lib_major_version == rhs.lib_major_version and lhs.lib_minor_version == rhs.lib_minor_version and From 22e6bbdb3a8344ab0c86b0bf81a1554971d934ef Mon Sep 17 00:00:00 2001 From: vuule Date: Tue, 18 Apr 2023 16:28:28 -0700 Subject: [PATCH 8/8] typo fix --- cpp/src/io/comp/nvcomp_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/io/comp/nvcomp_adapter.cpp b/cpp/src/io/comp/nvcomp_adapter.cpp index 6054e6dc892..68831ce1e83 100644 --- a/cpp/src/io/comp/nvcomp_adapter.cpp +++ b/cpp/src/io/comp/nvcomp_adapter.cpp @@ -449,7 +449,7 @@ feature_status_parameters::feature_status_parameters() cudaDeviceGetAttribute(&compute_capability_major, cudaDevAttrComputeCapabilityMajor, device)); } -// Represents all parameters reqired to determine status of a compression/decompression feature +// Represents all parameters required to determine status of a compression/decompression feature using feature_status_inputs = std::pair; struct hash_feature_status_inputs { size_t operator()(feature_status_inputs const& fsi) const