From 5deb96017db5488ad7c172fc2752b78bfb68cb62 Mon Sep 17 00:00:00 2001 From: vuule Date: Mon, 31 Jul 2023 15:03:49 -0700 Subject: [PATCH 01/11] register mmap --- cpp/src/io/utilities/datasource.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index 8aea8b4f69c..9028243a0eb 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -104,6 +104,24 @@ class file_source : public datasource { static constexpr size_t _gds_read_preferred_threshold = 128 << 10; // 128KB }; +void register_mmaped_buffer(void const* ptr, size_t size) +{ + if (ptr == nullptr or size != 0) { return; } + + int deviceId{}; + cudaGetDevice(&deviceId); + cudaDeviceProp deviceProp{}; + cudaGetDeviceProperties(&deviceProp, deviceId); + // Only register the memory mapped buffer if the device accesses pageable memory via the host's + // page tables - `cudaHostRegister` is very cheap in this case + if (deviceProp.pageableMemoryAccessUsesHostPageTables == 0) { return; } + + auto const result = cudaHostRegister(const_cast(ptr), size, cudaHostRegisterDefault); + if (result != cudaSuccess) { + CUDF_LOG_INFO("Failed to register the memory mapped buffer (possible performance impact)."); + } +} + /** * @brief Implementation class for reading from a file using memory mapped access. * @@ -162,6 +180,8 @@ class memory_mapped_source : public file_source { // Check if accessing a region within already mapped area _map_addr = mmap(nullptr, _map_size, PROT_READ, MAP_PRIVATE, fd, _map_offset); CUDF_EXPECTS(_map_addr != MAP_FAILED, "Cannot create memory mapping"); + + register_mmaped_buffer(_map_addr, _map_size); } private: From de299e4ac5e7171f65207a6c300c5078635a9dde Mon Sep 17 00:00:00 2001 From: vuule Date: Mon, 31 Jul 2023 15:53:37 -0700 Subject: [PATCH 02/11] spam --- cpp/src/io/utilities/datasource.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index 9028243a0eb..9d0dfb7e11c 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -106,7 +106,10 @@ class file_source : public datasource { void register_mmaped_buffer(void const* ptr, size_t size) { - if (ptr == nullptr or size != 0) { return; } + if (ptr == nullptr or size == 0) { + CUDF_LOG_WARN("Register mmap buffer: empty input"); + return; + } int deviceId{}; cudaGetDevice(&deviceId); @@ -114,12 +117,13 @@ void register_mmaped_buffer(void const* ptr, size_t size) cudaGetDeviceProperties(&deviceProp, deviceId); // Only register the memory mapped buffer if the device accesses pageable memory via the host's // page tables - `cudaHostRegister` is very cheap in this case - if (deviceProp.pageableMemoryAccessUsesHostPageTables == 0) { return; } + if (deviceProp.pageableMemoryAccessUsesHostPageTables == 0) { + CUDF_LOG_WARN("Register mmap buffer: pageableMemoryAccessUsesHostPageTables == 0"); + return; + } auto const result = cudaHostRegister(const_cast(ptr), size, cudaHostRegisterDefault); - if (result != cudaSuccess) { - CUDF_LOG_INFO("Failed to register the memory mapped buffer (possible performance impact)."); - } + if (result != cudaSuccess) { CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister failed"); } } /** From 484afc1c7351400caa3a0d5ec3e8ee6fd24ba1ac Mon Sep 17 00:00:00 2001 From: vuule Date: Mon, 31 Jul 2023 15:54:47 -0700 Subject: [PATCH 03/11] more spam --- cpp/src/io/utilities/datasource.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index 9d0dfb7e11c..f9e6d85380a 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -123,7 +123,11 @@ void register_mmaped_buffer(void const* ptr, size_t size) } auto const result = cudaHostRegister(const_cast(ptr), size, cudaHostRegisterDefault); - if (result != cudaSuccess) { CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister failed"); } + if (result != cudaSuccess) { + CUDF_LOG_WARN("Register mmap buffer: success"); + } else { + CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister failed"); + } } /** From b188ed0cb496f782fae1dcdfeea2319b5fdd8311 Mon Sep 17 00:00:00 2001 From: vuule Date: Wed, 2 Aug 2023 15:14:48 -0700 Subject: [PATCH 04/11] error code --- cpp/src/io/utilities/datasource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index f9e6d85380a..6a7a28c8b01 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -126,7 +126,7 @@ void register_mmaped_buffer(void const* ptr, size_t size) if (result != cudaSuccess) { CUDF_LOG_WARN("Register mmap buffer: success"); } else { - CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister failed"); + CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister failed with error {}", result); } } From 196f4903dc6eacbff941b71ccbc48c888f10ea7a Mon Sep 17 00:00:00 2001 From: vuule Date: Wed, 2 Aug 2023 16:48:41 -0700 Subject: [PATCH 05/11] omg --- cpp/src/io/utilities/datasource.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index 6a7a28c8b01..cde673eae00 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -123,11 +123,7 @@ void register_mmaped_buffer(void const* ptr, size_t size) } auto const result = cudaHostRegister(const_cast(ptr), size, cudaHostRegisterDefault); - if (result != cudaSuccess) { - CUDF_LOG_WARN("Register mmap buffer: success"); - } else { - CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister failed with error {}", result); - } + CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister returned {}", result); } /** From a1f4426a42e6949bd765df4a6bd8b4c4be9cc9f7 Mon Sep 17 00:00:00 2001 From: vuule Date: Thu, 3 Aug 2023 09:51:11 -0700 Subject: [PATCH 06/11] unregister --- cpp/src/io/utilities/datasource.cpp | 76 ++++++++++++++++++----------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index cde673eae00..3f013296b1f 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -104,28 +104,6 @@ class file_source : public datasource { static constexpr size_t _gds_read_preferred_threshold = 128 << 10; // 128KB }; -void register_mmaped_buffer(void const* ptr, size_t size) -{ - if (ptr == nullptr or size == 0) { - CUDF_LOG_WARN("Register mmap buffer: empty input"); - return; - } - - int deviceId{}; - cudaGetDevice(&deviceId); - cudaDeviceProp deviceProp{}; - cudaGetDeviceProperties(&deviceProp, deviceId); - // Only register the memory mapped buffer if the device accesses pageable memory via the host's - // page tables - `cudaHostRegister` is very cheap in this case - if (deviceProp.pageableMemoryAccessUsesHostPageTables == 0) { - CUDF_LOG_WARN("Register mmap buffer: pageableMemoryAccessUsesHostPageTables == 0"); - return; - } - - auto const result = cudaHostRegister(const_cast(ptr), size, cudaHostRegisterDefault); - CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister returned {}", result); -} - /** * @brief Implementation class for reading from a file using memory mapped access. * @@ -137,12 +115,18 @@ class memory_mapped_source : public file_source { explicit memory_mapped_source(char const* filepath, size_t offset, size_t size) : file_source(filepath) { - if (_file.size() != 0) map(_file.desc(), offset, size); + if (_file.size() != 0) { + map(_file.desc(), offset, size); + register_mmaped_buffer(); + } } ~memory_mapped_source() override { - if (_map_addr != nullptr) { munmap(_map_addr, _map_size); } + if (_map_addr != nullptr) { + munmap(_map_addr, _map_size); + unregister_mmaped_buffer(); + } } std::unique_ptr host_read(size_t offset, size_t size) override @@ -169,6 +153,41 @@ class memory_mapped_source : public file_source { } private: + void register_mmaped_buffer() + { + if (_map_addr == nullptr or _map_size == 0) { + CUDF_LOG_WARN("Register mmap buffer: empty input"); + return; + } + + int deviceId{}; + cudaGetDevice(&deviceId); + cudaDeviceProp deviceProp{}; + cudaGetDeviceProperties(&deviceProp, deviceId); + // Only register the memory mapped buffer if the device accesses pageable memory via the host's + // page tables - `cudaHostRegister` is very cheap in this case + if (deviceProp.pageableMemoryAccessUsesHostPageTables == 0) { + CUDF_LOG_WARN("Register mmap buffer: pageableMemoryAccessUsesHostPageTables == 0"); + return; + } + + auto const result = + cudaHostRegister(const_cast(_map_addr), _map_size, cudaHostRegisterDefault); + CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister returned {}", result); + is_registered = (result == cudaSuccess); + } + + void unregister_mmaped_buffer() + { + if (not is_registered) { + CUDF_LOG_WARN("Unregister mmap buffer: not registered"); + return; + } + + auto const result = cudaHostUnregister(const_cast(_map_addr)); + CUDF_LOG_WARN("Unregister mmap buffer: cudaHostUnregister returned {}", result); + } + void map(int fd, size_t offset, size_t size) { CUDF_EXPECTS(offset < _file.size(), "Offset is past end of file"); @@ -184,14 +203,13 @@ class memory_mapped_source : public file_source { // Check if accessing a region within already mapped area _map_addr = mmap(nullptr, _map_size, PROT_READ, MAP_PRIVATE, fd, _map_offset); CUDF_EXPECTS(_map_addr != MAP_FAILED, "Cannot create memory mapping"); - - register_mmaped_buffer(_map_addr, _map_size); } private: - size_t _map_size = 0; - size_t _map_offset = 0; - void* _map_addr = nullptr; + size_t _map_size = 0; + size_t _map_offset = 0; + void* _map_addr = nullptr; + bool _is_registered = false; }; /** From 16818feec602be94eafecb5cecda2ed2ffcb155f Mon Sep 17 00:00:00 2001 From: vuule Date: Thu, 3 Aug 2023 12:14:42 -0700 Subject: [PATCH 07/11] typo --- cpp/src/io/utilities/datasource.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index 3f013296b1f..a7bf6ff9f42 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -174,12 +174,12 @@ class memory_mapped_source : public file_source { auto const result = cudaHostRegister(const_cast(_map_addr), _map_size, cudaHostRegisterDefault); CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister returned {}", result); - is_registered = (result == cudaSuccess); + _is_registered = (result == cudaSuccess); } void unregister_mmaped_buffer() { - if (not is_registered) { + if (not _is_registered) { CUDF_LOG_WARN("Unregister mmap buffer: not registered"); return; } From f3385f6230b2c4694e53641edd768e1ec5c110e4 Mon Sep 17 00:00:00 2001 From: vuule Date: Thu, 3 Aug 2023 13:09:19 -0700 Subject: [PATCH 08/11] property memoization --- cpp/src/io/utilities/datasource.cpp | 61 +++++++++++++++++------------ 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index a7bf6ff9f42..8b98d5b6d09 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -29,6 +29,8 @@ #include #include +#include + namespace cudf { namespace io { namespace { @@ -104,6 +106,24 @@ class file_source : public datasource { static constexpr size_t _gds_read_preferred_threshold = 128 << 10; // 128KB }; +[[nodiscard]] bool pageableMemoryAccessUsesHostPageTables() +{ + static std::unordered_map result_cache{}; + + int deviceId{}; + cudaGetDevice(&deviceId); + + if (result_cache.find(deviceId) == result_cache.end()) { + cudaDeviceProp props{}; + cudaGetDeviceProperties(&props, deviceId); + result_cache[deviceId] = (props.pageableMemoryAccessUsesHostPageTables == 1); + CUDF_LOG_INFO( + "Device {} pageableMemoryAccessUsesHostPageTables: {}", deviceId, result_cache[deviceId]); + } + + return result_cache[deviceId]; +} + /** * @brief Implementation class for reading from a file using memory mapped access. * @@ -155,37 +175,26 @@ class memory_mapped_source : public file_source { private: void register_mmaped_buffer() { - if (_map_addr == nullptr or _map_size == 0) { - CUDF_LOG_WARN("Register mmap buffer: empty input"); + if (_map_addr == nullptr or _map_size == 0 or not pageableMemoryAccessUsesHostPageTables()) { return; } - int deviceId{}; - cudaGetDevice(&deviceId); - cudaDeviceProp deviceProp{}; - cudaGetDeviceProperties(&deviceProp, deviceId); - // Only register the memory mapped buffer if the device accesses pageable memory via the host's - // page tables - `cudaHostRegister` is very cheap in this case - if (deviceProp.pageableMemoryAccessUsesHostPageTables == 0) { - CUDF_LOG_WARN("Register mmap buffer: pageableMemoryAccessUsesHostPageTables == 0"); - return; + auto const result = cudaHostRegister(_map_addr, _map_size, cudaHostRegisterDefault); + if (result == cudaSuccess) { + _is_map_registered = true; + } else { + CUDF_LOG_WARN("cudaHostRegister failed with {} ({})", result, cudaGetErrorString(result)); } - - auto const result = - cudaHostRegister(const_cast(_map_addr), _map_size, cudaHostRegisterDefault); - CUDF_LOG_WARN("Register mmap buffer: cudaHostRegister returned {}", result); - _is_registered = (result == cudaSuccess); } void unregister_mmaped_buffer() { - if (not _is_registered) { - CUDF_LOG_WARN("Unregister mmap buffer: not registered"); - return; - } + if (not _is_map_registered) { return; } - auto const result = cudaHostUnregister(const_cast(_map_addr)); - CUDF_LOG_WARN("Unregister mmap buffer: cudaHostUnregister returned {}", result); + auto const result = cudaHostUnregister(_map_addr); + if (result != cudaSuccess) { + CUDF_LOG_WARN("cudaHostUnregister failed with {} ({})", result, cudaGetErrorString(result)); + } } void map(int fd, size_t offset, size_t size) @@ -206,10 +215,10 @@ class memory_mapped_source : public file_source { } private: - size_t _map_size = 0; - size_t _map_offset = 0; - void* _map_addr = nullptr; - bool _is_registered = false; + size_t _map_size = 0; + size_t _map_offset = 0; + void* _map_addr = nullptr; + bool _is_map_registered = false; }; /** From 0c15955db5ed522a15e592231505703c65d256f8 Mon Sep 17 00:00:00 2001 From: vuule Date: Mon, 7 Aug 2023 10:33:35 -0700 Subject: [PATCH 09/11] docs --- cpp/src/io/utilities/datasource.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index 8b98d5b6d09..a93b04edc89 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -106,6 +106,9 @@ class file_source : public datasource { static constexpr size_t _gds_read_preferred_threshold = 128 << 10; // 128KB }; +/** + * @brief Memoized pageableMemoryAccessUsesHostPageTables device property. + */ [[nodiscard]] bool pageableMemoryAccessUsesHostPageTables() { static std::unordered_map result_cache{}; @@ -173,6 +176,11 @@ class memory_mapped_source : public file_source { } private: + /** + * @brief Page-locks (registers) the memory range of the mapped file. + * + * Fixes nvbugs/4215160 + */ void register_mmaped_buffer() { if (_map_addr == nullptr or _map_size == 0 or not pageableMemoryAccessUsesHostPageTables()) { @@ -187,6 +195,9 @@ class memory_mapped_source : public file_source { } } + /** + * @brief Unregisters the memory range of the mapped file. + */ void unregister_mmaped_buffer() { if (not _is_map_registered) { return; } From cdea40756cc85b3a00acece963ce2fb7d13a934c Mon Sep 17 00:00:00 2001 From: vuule Date: Mon, 7 Aug 2023 10:43:45 -0700 Subject: [PATCH 10/11] typo fix --- cpp/src/io/utilities/datasource.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index a93b04edc89..d069488a567 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -140,7 +140,7 @@ class memory_mapped_source : public file_source { { if (_file.size() != 0) { map(_file.desc(), offset, size); - register_mmaped_buffer(); + register_mmap_buffer(); } } @@ -148,7 +148,7 @@ class memory_mapped_source : public file_source { { if (_map_addr != nullptr) { munmap(_map_addr, _map_size); - unregister_mmaped_buffer(); + unregister_mmap_buffer(); } } @@ -181,7 +181,7 @@ class memory_mapped_source : public file_source { * * Fixes nvbugs/4215160 */ - void register_mmaped_buffer() + void register_mmap_buffer() { if (_map_addr == nullptr or _map_size == 0 or not pageableMemoryAccessUsesHostPageTables()) { return; @@ -198,7 +198,7 @@ class memory_mapped_source : public file_source { /** * @brief Unregisters the memory range of the mapped file. */ - void unregister_mmaped_buffer() + void unregister_mmap_buffer() { if (not _is_map_registered) { return; } From cd025b839ef3a74d191cd272d4dc6a3b7249539c Mon Sep 17 00:00:00 2001 From: Vukasin Milovanovic Date: Thu, 10 Aug 2023 09:14:43 -0700 Subject: [PATCH 11/11] Add CUDF_CUDA_TRYs Co-authored-by: David Wendt <45795991+davidwendt@users.noreply.github.com> --- cpp/src/io/utilities/datasource.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/io/utilities/datasource.cpp b/cpp/src/io/utilities/datasource.cpp index d069488a567..9a5df215c90 100644 --- a/cpp/src/io/utilities/datasource.cpp +++ b/cpp/src/io/utilities/datasource.cpp @@ -114,11 +114,11 @@ class file_source : public datasource { static std::unordered_map result_cache{}; int deviceId{}; - cudaGetDevice(&deviceId); + CUDF_CUDA_TRY(cudaGetDevice(&deviceId)); if (result_cache.find(deviceId) == result_cache.end()) { cudaDeviceProp props{}; - cudaGetDeviceProperties(&props, deviceId); + CUDF_CUDA_TRY(cudaGetDeviceProperties(&props, deviceId)); result_cache[deviceId] = (props.pageableMemoryAccessUsesHostPageTables == 1); CUDF_LOG_INFO( "Device {} pageableMemoryAccessUsesHostPageTables: {}", deviceId, result_cache[deviceId]);