Skip to content

Commit

Permalink
Remove the additional host register calls initially intended for perf…
Browse files Browse the repository at this point in the history
…ormance improvement on Grace Hopper
  • Loading branch information
kingcrimsontianyu committed Oct 15, 2024
1 parent 319ec3b commit 5133fcb
Showing 1 changed file with 1 addition and 69 deletions.
70 changes: 1 addition & 69 deletions cpp/src/io/utilities/datasource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,27 +134,6 @@ 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<int, bool> result_cache{};

int deviceId{};
CUDF_CUDA_TRY(cudaGetDevice(&deviceId));

if (result_cache.find(deviceId) == result_cache.end()) {
cudaDeviceProp props{};
CUDF_CUDA_TRY(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.
*
Expand All @@ -172,19 +151,12 @@ class memory_mapped_source : public file_source {
if (_file.size() != 0) {
// Memory mapping is not exclusive, so we can include the whole region we expect to read
map(_file.desc(), offset, max_size_estimate);
// Buffer registration is exclusive (can't overlap with other registered buffers) so we
// register the lower estimate; this avoids issues when reading adjacent ranges from the same
// file from multiple threads
register_mmap_buffer(offset, min_size_estimate);
}
}

~memory_mapped_source() override
{
if (_map_addr != nullptr) {
unmap();
unregister_mmap_buffer();
}
if (_map_addr != nullptr) { unmap(); }
}

std::unique_ptr<buffer> host_read(size_t offset, size_t size) override
Expand Down Expand Up @@ -227,46 +199,6 @@ class memory_mapped_source : public file_source {
}

private:
/**
* @brief Page-locks (registers) the memory range of the mapped file.
*
* Fixes nvbugs/4215160
*/
void register_mmap_buffer(size_t offset, size_t size)
{
if (_map_addr == nullptr or not pageableMemoryAccessUsesHostPageTables()) { return; }

// Registered region must be within the mapped region
_reg_offset = std::max(offset, _map_offset);
_reg_size = std::min(size != 0 ? size : _map_size, (_map_offset + _map_size) - _reg_offset);

_reg_addr = static_cast<std::byte*>(_map_addr) - _map_offset + _reg_offset;
auto const result = cudaHostRegister(_reg_addr, _reg_size, cudaHostRegisterReadOnly);
if (result != cudaSuccess) {
_reg_addr = nullptr;
CUDF_LOG_WARN("cudaHostRegister failed with {} ({})",
static_cast<int>(result),
cudaGetErrorString(result));
}
}

/**
* @brief Unregisters the memory range of the mapped file.
*/
void unregister_mmap_buffer()
{
if (_reg_addr == nullptr) { return; }

auto const result = cudaHostUnregister(_reg_addr);
if (result == cudaSuccess) {
_reg_addr = nullptr;
} else {
CUDF_LOG_WARN("cudaHostUnregister failed with {} ({})",
static_cast<int>(result),
cudaGetErrorString(result));
}
}

void map(int fd, size_t offset, size_t size)
{
CUDF_EXPECTS(offset < _file.size(), "Offset is past end of file", std::overflow_error);
Expand Down

0 comments on commit 5133fcb

Please sign in to comment.