-
Notifications
You must be signed in to change notification settings - Fork 201
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 Statistics Resource Adaptor and cython bindings to tracking_resource_adaptor
and statistics_resource_adaptor
#626
Merged
rapids-bot
merged 28 commits into
rapidsai:branch-21.08
from
mdemoret-nv:enh-extend-tracking-resource-adaptor
Jun 8, 2021
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
c5e91d9
Adding additional tracking info and cython bindings to tracking_resou…
mdemoret-nv b31bb2f
Adding PR to CHANGELOG
mdemoret-nv 533fe2a
Style cleanup
mdemoret-nv cffe9bf
Fixing incorrect Cython class name
mdemoret-nv e1218fa
Apply suggestions from code review
mdemoret-nv 25d5da3
Merge remote-tracking branch 'upstream/branch-0.17' into enh-extend-t…
mdemoret-nv 976dabb
Removed the reset() method, added ability to push/pull
mdemoret-nv 39d5e22
Merge remote-tracking branch 'upstream/branch-0.17' into enh-extend-t…
mdemoret-nv ddd296a
Style cleanup
mdemoret-nv df0054e
Adding a reference to the MR used for allocation in device_buffer
mdemoret-nv cbf2772
Merge remote-tracking branch 'upstream/branch-0.18' into enh-extend-t…
mdemoret-nv e3e586a
Removing reset() and push/pop from the tracking manager
mdemoret-nv da25869
Apply suggestions from code review
mdemoret-nv 312ca50
Changing ssize_t to int64_t per review from harrism
mdemoret-nv 4c677f5
Merge branch 'branch-0.20' into enh-extend-tracking-resource-adaptor
mdemoret-nv 1d64c6d
Incorporating feedback from code review. Simplifying counter struct.
mdemoret-nv 97753ee
Style cleanup.
mdemoret-nv b812bc0
Style cleanup for `black` which was missed in the logs
mdemoret-nv 9d74e5d
Cleaning up code to reduce number of changes with 0.20
mdemoret-nv 9e69c67
Update python/rmm/tests/test_rmm.py
mdemoret-nv 8bfd27b
Merge branch 'branch-0.20' into enh-extend-tracking-resource-adaptor
mdemoret-nv 7cf3123
Moving the dl library link to the `rmm::rmm` main interface.
mdemoret-nv b5bbab4
Merge branch 'branch-21.08' into enh-extend-tracking-resource-adaptor
mdemoret-nv c7395e4
Separated the statistics portion from the tracking_resource_adaptor i…
mdemoret-nv 1872ee2
Getting tests to pass
mdemoret-nv 6b20fb8
Style cleanup
mdemoret-nv b281f9e
Improving method comment.
mdemoret-nv cab217d
Style cleanup.
mdemoret-nv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,246 @@ | ||
/* | ||
* Copyright (c) 2020, NVIDIA CORPORATION. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
#pragma once | ||
|
||
#include <mutex> | ||
#include <rmm/mr/device/device_memory_resource.hpp> | ||
#include <shared_mutex> | ||
|
||
namespace rmm { | ||
namespace mr { | ||
/** | ||
* @brief Resource that uses `Upstream` to allocate memory and tracks statistics | ||
* on memory allocations. | ||
* | ||
* An instance of this resource can be constructed with an existing, upstream | ||
* resource in order to satisfy allocation requests, but any existing | ||
* allocations will be untracked. Tracking statistics stores the current, peak | ||
* and total memory allocations for both the number of bytes and number of calls | ||
* to the memory resource. `statistics_resource_adaptor` is intended as a debug | ||
* adaptor and shouldn't be used in performance-sensitive code. | ||
* | ||
* @tparam Upstream Type of the upstream resource used for | ||
* allocation/deallocation. | ||
*/ | ||
template <typename Upstream> | ||
class statistics_resource_adaptor final : public device_memory_resource { | ||
public: | ||
// can be a std::shared_mutex once C++17 is adopted | ||
using read_lock_t = std::shared_lock<std::shared_timed_mutex>; | ||
using write_lock_t = std::unique_lock<std::shared_timed_mutex>; | ||
|
||
/** | ||
* @brief Utility struct for counting the current, peak, and total value of a number | ||
*/ | ||
struct counter { | ||
int64_t value{0}; // Current value | ||
int64_t peak{0}; // Max value of `value` | ||
int64_t total{0}; // Sum of all added values | ||
|
||
counter& operator+=(int64_t x) | ||
{ | ||
value += x; | ||
total += x; | ||
peak = std::max(value, peak); | ||
return *this; | ||
} | ||
|
||
counter& operator-=(int64_t x) | ||
{ | ||
value -= x; | ||
return *this; | ||
} | ||
}; | ||
|
||
/** | ||
* @brief Construct a new statistics resource adaptor using `upstream` to satisfy | ||
* allocation requests. | ||
* | ||
* @throws `rmm::logic_error` if `upstream == nullptr` | ||
* | ||
* @param upstream The resource used for allocating/deallocating device memory | ||
*/ | ||
statistics_resource_adaptor(Upstream* upstream) : upstream_{upstream} | ||
{ | ||
RMM_EXPECTS(nullptr != upstream, "Unexpected null upstream resource pointer."); | ||
} | ||
|
||
statistics_resource_adaptor() = delete; | ||
virtual ~statistics_resource_adaptor() = default; | ||
statistics_resource_adaptor(statistics_resource_adaptor const&) = delete; | ||
statistics_resource_adaptor(statistics_resource_adaptor&&) = default; | ||
statistics_resource_adaptor& operator=(statistics_resource_adaptor const&) = delete; | ||
statistics_resource_adaptor& operator=(statistics_resource_adaptor&&) = default; | ||
|
||
/** | ||
* @brief Return pointer to the upstream resource. | ||
* | ||
* @return Upstream* Pointer to the upstream resource. | ||
*/ | ||
Upstream* get_upstream() const noexcept { return upstream_; } | ||
|
||
/** | ||
* @brief Checks whether the upstream resource supports streams. | ||
* | ||
* @return true The upstream resource supports streams | ||
* @return false The upstream resource does not support streams. | ||
*/ | ||
bool supports_streams() const noexcept override { return upstream_->supports_streams(); } | ||
|
||
/** | ||
* @brief Query whether the resource supports the get_mem_info API. | ||
* | ||
* @return bool true if the upstream resource supports get_mem_info, false otherwise. | ||
*/ | ||
bool supports_get_mem_info() const noexcept override | ||
{ | ||
return upstream_->supports_get_mem_info(); | ||
} | ||
|
||
/** | ||
* @brief Returns a `counter` struct for this adaptor containing the current, | ||
* peak, and total number of allocated bytes for this | ||
* adaptor since it was created. | ||
* | ||
* @return counter struct containing bytes count | ||
*/ | ||
counter get_bytes_counter() const noexcept | ||
{ | ||
read_lock_t lock(mtx_); | ||
|
||
return bytes_; | ||
} | ||
|
||
/** | ||
* @brief Returns a `counter` struct for this adaptor containing the current, | ||
* peak, and total number of allocation counts for this adaptor since it was | ||
* created. | ||
* | ||
* @return counter struct containing allocations count | ||
*/ | ||
counter get_allocations_counter() const noexcept | ||
{ | ||
read_lock_t lock(mtx_); | ||
|
||
return allocations_; | ||
} | ||
|
||
private: | ||
/** | ||
* @brief Allocates memory of size at least `bytes` using the upstream | ||
* resource as long as it fits inside the allocation limit. | ||
* | ||
* The returned pointer has at least 256B alignment. | ||
* | ||
* @throws `rmm::bad_alloc` if the requested allocation could not be fulfilled | ||
* by the upstream resource. | ||
* | ||
* @param bytes The size, in bytes, of the allocation | ||
* @param stream Stream on which to perform the allocation | ||
* @return void* Pointer to the newly allocated memory | ||
*/ | ||
void* do_allocate(std::size_t bytes, cuda_stream_view stream) override | ||
{ | ||
void* p = upstream_->allocate(bytes, stream); | ||
|
||
// increment the stats | ||
{ | ||
write_lock_t lock(mtx_); | ||
|
||
// Increment the allocation_count_ while we have the lock | ||
bytes_ += bytes; | ||
allocations_ += 1; | ||
} | ||
|
||
return p; | ||
} | ||
|
||
/** | ||
* @brief Free allocation of size `bytes` pointed to by `p` | ||
* | ||
* @throws Nothing. | ||
* | ||
* @param p Pointer to be deallocated | ||
* @param bytes Size of the allocation | ||
* @param stream Stream on which to perform the deallocation | ||
*/ | ||
void do_deallocate(void* p, std::size_t bytes, cuda_stream_view stream) override | ||
{ | ||
upstream_->deallocate(p, bytes, stream); | ||
|
||
{ | ||
write_lock_t lock(mtx_); | ||
|
||
// Decrement the current allocated counts. | ||
bytes_ -= bytes; | ||
allocations_ -= 1; | ||
} | ||
} | ||
|
||
/** | ||
* @brief Compare the upstream resource to another. | ||
* | ||
* @throws Nothing. | ||
* | ||
* @param other The other resource to compare to | ||
* @return true If the two resources are equivalent | ||
* @return false If the two resources are not equal | ||
*/ | ||
bool do_is_equal(device_memory_resource const& other) const noexcept override | ||
{ | ||
if (this == &other) | ||
return true; | ||
else { | ||
auto cast = dynamic_cast<statistics_resource_adaptor<Upstream> const*>(&other); | ||
return cast != nullptr ? upstream_->is_equal(*cast->get_upstream()) | ||
: upstream_->is_equal(other); | ||
} | ||
} | ||
|
||
/** | ||
* @brief Get free and available memory from upstream resource. | ||
* | ||
* @throws `rmm::cuda_error` if unable to retrieve memory info. | ||
* | ||
* @param stream Stream on which to get the mem info. | ||
* @return std::pair contaiing free_size and total_size of memory | ||
*/ | ||
std::pair<std::size_t, std::size_t> do_get_mem_info(cuda_stream_view stream) const override | ||
{ | ||
return upstream_->get_mem_info(stream); | ||
} | ||
|
||
counter bytes_; // peak, current and total allocated bytes | ||
counter allocations_; // peak, current and total allocation count | ||
std::shared_timed_mutex mutable mtx_; // mutex for thread safe access to allocations_ | ||
Upstream* upstream_; // the upstream resource used for satisfying allocation requests | ||
}; | ||
|
||
/** | ||
* @brief Convenience factory to return a `statistics_resource_adaptor` around the | ||
* upstream resource `upstream`. | ||
* | ||
* @tparam Upstream Type of the upstream `device_memory_resource`. | ||
* @param upstream Pointer to the upstream resource | ||
*/ | ||
template <typename Upstream> | ||
statistics_resource_adaptor<Upstream> make_statistics_adaptor(Upstream* upstream) | ||
{ | ||
return statistics_resource_adaptor<Upstream>{upstream}; | ||
} | ||
|
||
} // namespace mr | ||
} // namespace rmm |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -81,7 +81,7 @@ class tracking_resource_adaptor final : public device_memory_resource { | |||||
} | ||||||
|
||||||
tracking_resource_adaptor() = delete; | ||||||
~tracking_resource_adaptor() = default; | ||||||
virtual ~tracking_resource_adaptor() = default; | ||||||
tracking_resource_adaptor(tracking_resource_adaptor const&) = delete; | ||||||
tracking_resource_adaptor(tracking_resource_adaptor&&) = default; | ||||||
tracking_resource_adaptor& operator=(tracking_resource_adaptor const&) = delete; | ||||||
|
@@ -136,24 +136,42 @@ class tracking_resource_adaptor final : public device_memory_resource { | |||||
std::size_t get_allocated_bytes() const noexcept { return allocated_bytes_; } | ||||||
|
||||||
/** | ||||||
* @brief Log any outstanding allocations via RMM_LOG_DEBUG | ||||||
* @brief Gets a string containing the outstanding allocation pointers, their | ||||||
* size, and optionally the stack trace for when each pointer was allocated. | ||||||
mdemoret-nv marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
* | ||||||
* Stack traces are only included if this resource adaptor was created with | ||||||
* `capture_stack == true`. Otherwise, outstanding allocation pointers will be | ||||||
* shown with their size and empty stack traces. | ||||||
* | ||||||
* @return std::string Containing the outstanding allocation pointers. | ||||||
*/ | ||||||
void log_outstanding_allocations() const | ||||||
std::string get_outstanding_allocations_str() const | ||||||
{ | ||||||
#if SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG | ||||||
read_lock_t lock(mtx_); | ||||||
if (not allocations_.empty()) { | ||||||
std::ostringstream oss; | ||||||
|
||||||
std::ostringstream oss; | ||||||
|
||||||
if (!allocations_.empty()) { | ||||||
for (auto const& al : allocations_) { | ||||||
oss << al.first << ": " << al.second.allocation_size << " B"; | ||||||
if (al.second.strace != nullptr) { | ||||||
oss << " : callstack:" << std::endl << *al.second.strace; | ||||||
} | ||||||
oss << std::endl; | ||||||
} | ||||||
RMM_LOG_DEBUG("Outstanding Allocations: {}", oss.str()); | ||||||
} | ||||||
|
||||||
return oss.str(); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @brief Log any outstanding allocations via RMM_LOG_DEBUG | ||||||
* | ||||||
*/ | ||||||
void log_outstanding_allocations() const | ||||||
{ | ||||||
#if SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG | ||||||
RMM_LOG_DEBUG("Outstanding Allocations: {}", get_outstanding_allocations_str()); | ||||||
#endif // SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG | ||||||
} | ||||||
|
||||||
|
@@ -199,7 +217,33 @@ class tracking_resource_adaptor final : public device_memory_resource { | |||||
upstream_->deallocate(p, bytes, stream); | ||||||
{ | ||||||
write_lock_t lock(mtx_); | ||||||
allocations_.erase(p); | ||||||
|
||||||
const auto found = allocations_.find(p); | ||||||
|
||||||
// Ensure the allocation is found and the number of bytes match | ||||||
if (found == allocations_.end()) { | ||||||
// Don't throw but log an error. Throwing in a descructor (or any noexcept) will call | ||||||
// std::terminate | ||||||
RMM_LOG_ERROR( | ||||||
"Deallocating a pointer that was not tracked. Ptr: {:p} [{}B], Current Num. Allocations: " | ||||||
"{}", | ||||||
fmt::ptr(p), | ||||||
bytes, | ||||||
this->allocations_.size()); | ||||||
} else { | ||||||
allocations_.erase(found); | ||||||
|
||||||
auto allocated_bytes = found->second.allocation_size; | ||||||
|
||||||
if (allocated_bytes != bytes) { | ||||||
// Don't throw but log an error. Throwing in a descructor (or any noexcept) will call | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// std::terminate | ||||||
RMM_LOG_ERROR( | ||||||
"Alloc bytes ({}) and Dealloc bytes ({}) do not match", allocated_bytes, bytes); | ||||||
|
||||||
bytes = allocated_bytes; | ||||||
} | ||||||
} | ||||||
} | ||||||
allocated_bytes_ -= bytes; | ||||||
} | ||||||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? Quick google shows that dladdr now lives in libc rather than libdl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With stack traces enabled, this was needed to compile the tests (original comment). Keith and I briefly discussed this here: #626 (comment).
Can you send me the link where you saw that
dladdr
has moved? All I am seeing from this link is:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, the docs I found were not for linux -- Solaris and something called illumos. As I said, it was a quick google.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested that all is well in libcudf, cuML, etc. when this library is linked here? Note that the other
target_link_libraries
for RMM are all header-only, which is why this one has me worried (RMM is a header-only library).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdemoret reports cuML builds and tests fine against this PR.