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

Improve C++ Test Coverage #920

Merged
merged 36 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
809a6d4
Add thread_safe_adaptor tests
harrism Nov 11, 2021
9894edc
Improve cuda_stream test coverage
harrism Nov 11, 2021
d2d9e5a
Avoid assertion in debug mode and add test of operator<<
harrism Nov 16, 2021
7874870
Test coverage of e.what()
harrism Nov 16, 2021
c7c783e
Improve arena_mr tests
harrism Nov 17, 2021
665ba0c
Test device_buffer::is_empty()
harrism Nov 17, 2021
6606b63
Add generic adaptor tests
harrism Nov 17, 2021
c42a10c
Merge branch 'branch-22.02' into fea-improve-codecov
harrism Nov 17, 2021
b69d412
Delete accidentally added logfile
harrism Nov 17, 2021
4034dd6
"out_of_memory" in rmm::out_of_memory.what()
harrism Nov 18, 2021
3525955
Test owning_wrapper in adaptor_tests
harrism Nov 18, 2021
c0db7df
Test upstream failure in limiting_mr
harrism Nov 18, 2021
a1479fc
cmake style
harrism Nov 18, 2021
2323b33
Improve test coverage of fixed-size MR and pool MR
harrism Nov 18, 2021
fefd26a
Disable debugging code
harrism Nov 23, 2021
cd0b042
Add explicit instantiations to improve coverage (force instantiation …
harrism Nov 23, 2021
b39b2ed
Improve thrust_allocator test coverage
harrism Nov 23, 2021
85b31a9
Improve device_scalar test coverage
harrism Nov 23, 2021
3b24799
Comment out debug helper print, and use get_upstream() instead of acc…
harrism Nov 23, 2021
4f95f51
simpler
harrism Nov 23, 2021
8be0df4
Improve device_uvector coverage
harrism Nov 23, 2021
98e7581
Cover NE adaptor comparison
harrism Nov 23, 2021
b9ce8a7
Improve binning_mr coverage
harrism Nov 23, 2021
1121b40
Move list debugging code behind a macro, and remove whitebox tests.
harrism Nov 23, 2021
5860bce
Explicitly test stream_view dtor
harrism Nov 23, 2021
98c9d50
Explanatory Comment
harrism Nov 23, 2021
62e23cc
Improve logger coverage.
harrism Nov 23, 2021
c1fd31f
cmake style
harrism Nov 23, 2021
02744af
Add test of log_outstanding_allocations()
harrism Nov 24, 2021
48aabba
stack_trace expects non-null strings. Fixes uncovered branch.
harrism Nov 24, 2021
6c76d49
Remove unneeded flush.
harrism Nov 24, 2021
3c45415
shorten lines to improve coverage.
harrism Nov 24, 2021
9a24828
Clean up logger construction to improve coverage
harrism Nov 24, 2021
de4d818
Improve arena.hpp coverage by splitting out lambda.
harrism Nov 24, 2021
04906a5
Use CTAD on std::array.
harrism Nov 30, 2021
5096e16
Update include/rmm/mr/device/pool_memory_resource.hpp
harrism Nov 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions gcovr.cfg
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
exclude=build/.*
exclude=tests/.*
exclude=benchmarks/.*
html=yes
html-details=yes
sort-percentage=yes
exclude-throw-branches=yes
5 changes: 3 additions & 2 deletions include/rmm/detail/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct cuda_error : public std::runtime_error {
class bad_alloc : public std::bad_alloc {
public:
bad_alloc(const char* msg) : _what{std::string{std::bad_alloc::what()} + ": " + msg} {}
bad_alloc(std::string const& msg) : bad_alloc(msg.c_str()) {}
bad_alloc(std::string const& msg) : bad_alloc{msg.c_str()} {}

[[nodiscard]] const char* what() const noexcept override { return _what.c_str(); }

Expand All @@ -67,7 +67,8 @@ class bad_alloc : public std::bad_alloc {
*/
class out_of_memory : public bad_alloc {
public:
using bad_alloc::bad_alloc;
out_of_memory(const char* msg) : bad_alloc{std::string{"out_of_memory: "} + msg} {}
out_of_memory(std::string const& msg) : out_of_memory{msg.c_str()} {}
};

/**
Expand Down
43 changes: 21 additions & 22 deletions include/rmm/detail/stack_trace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#pragma once

#include <rmm/detail/error.hpp>

// execinfo is a linux-only library, so stack traces will only be available on
// linux systems.
#if (defined(__GNUC__) && !defined(__MINGW32__) && !defined(__MINGW64__))
Expand Down Expand Up @@ -64,32 +66,29 @@ class stack_trace {
backtrace_symbols(trace.stack_ptrs.data(), static_cast<int>(trace.stack_ptrs.size())),
&::free);

if (strings == nullptr) {
os << "But no stack trace could be found!" << std::endl;
} else {
// Iterate over the stack pointers converting to a string
for (std::size_t i = 0; i < trace.stack_ptrs.size(); ++i) {
// Leading index
os << "#" << i << " in ";
RMM_EXPECTS(strings != nullptr, "Unexpected null stack trace symbols");
// Iterate over the stack pointers converting to a string
for (std::size_t i = 0; i < trace.stack_ptrs.size(); ++i) {
// Leading index
os << "#" << i << " in ";

auto const str = [&] {
Dl_info info;
if (dladdr(trace.stack_ptrs[i], &info) != 0) {
int status = -1; // Demangle the name. This can occasionally fail
auto const str = [&] {
Dl_info info;
if (dladdr(trace.stack_ptrs[i], &info) != 0) {
int status = -1; // Demangle the name. This can occasionally fail

std::unique_ptr<char, decltype(&::free)> demangled(
abi::__cxa_demangle(info.dli_sname, nullptr, nullptr, &status), &::free);
// If it fails, fallback to the dli_name.
if (status == 0 or (info.dli_sname != nullptr)) {
auto const* name = status == 0 ? demangled.get() : info.dli_sname;
return name + std::string(" from ") + info.dli_fname;
}
std::unique_ptr<char, decltype(&::free)> demangled(
abi::__cxa_demangle(info.dli_sname, nullptr, nullptr, &status), &::free);
// If it fails, fallback to the dli_name.
if (status == 0 or (info.dli_sname != nullptr)) {
auto const* name = status == 0 ? demangled.get() : info.dli_sname;
return name + std::string(" from ") + info.dli_fname;
}
return std::string(strings.get()[i]);
}();
}
return std::string(strings.get()[i]);
}();

os << str << std::endl;
}
os << str << std::endl;
}
#else
os << "stack traces disabled" << std::endl;
Expand Down
4 changes: 2 additions & 2 deletions include/rmm/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ struct bytes {

friend std::ostream& operator<<(std::ostream& os, bytes const& value)
{
static std::array<std::string, 9> const units{
"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"};
static std::array units{"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"};

int index = 0;
auto size = static_cast<double>(value.value);
while (size > 1024) {
Expand Down
1 change: 1 addition & 0 deletions include/rmm/mr/device/arena_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ class arena_memory_resource final : public device_memory_resource {
stream_arena.second.dump_memory_log(logger_);
}
}
logger_->flush();
}

/**
Expand Down
5 changes: 3 additions & 2 deletions include/rmm/mr/device/detail/arena.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,9 @@ constexpr std::size_t align_down(std::size_t value) noexcept
*/
inline block first_fit(std::set<block>& free_blocks, std::size_t size)
{
auto const iter = std::find_if(
free_blocks.cbegin(), free_blocks.cend(), [size](auto const& blk) { return blk.fits(size); });
auto fits = [size](auto const& blk) { return blk.fits(size); };

auto const iter = std::find_if(free_blocks.cbegin(), free_blocks.cend(), fits);

if (iter == free_blocks.cend()) { return {}; }
// Remove the block from the free_list.
Expand Down
21 changes: 14 additions & 7 deletions include/rmm/mr/device/detail/coalescing_free_list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#pragma once

#include <iterator>
#include <rmm/detail/error.hpp>
#include <rmm/mr/device/detail/free_list.hpp>

Expand Down Expand Up @@ -122,25 +123,29 @@ struct block : public block_base {
return fits(bytes) && (size() < blk.size() || blk.size() < bytes);
}

#ifdef RMM_DEBUG_PRINT
/**
* @brief Print this block. For debugging.
*/
inline void print() const
{
std::cout << fmt::format("{} {} B", fmt::ptr(pointer()), size()) << std::endl;
}
#endif

private:
std::size_t size_bytes{}; ///< Size in bytes
bool head{}; ///< Indicates whether ptr was allocated from the heap
};

#ifdef RMM_DEBUG_PRINT
/// Print block on an ostream
inline std::ostream& operator<<(std::ostream& out, const block& blk)
{
out << fmt::format("{} {} B\n", fmt::ptr(blk.pointer()), blk.size());
return out;
}
#endif

/**
* @brief Comparator for block types based on pointer address.
Expand Down Expand Up @@ -217,9 +222,9 @@ struct coalescing_free_list : free_list<block> {
*/
void insert(free_list&& other)
{
std::for_each(std::make_move_iterator(other.begin()),
std::make_move_iterator(other.end()),
[this](block_type&& block) { this->insert(block); });
using std::make_move_iterator;
auto inserter = [this](block_type&& block) { this->insert(block); };
std::for_each(make_move_iterator(other.begin()), make_move_iterator(other.end()), inserter);
}

/**
Expand All @@ -233,10 +238,10 @@ struct coalescing_free_list : free_list<block> {
block_type get_block(std::size_t size)
{
// find best fit block
auto const iter =
std::min_element(cbegin(), cend(), [size](block_type const& lhs, block_type const& rhs) {
return lhs.is_better_fit(size, rhs);
});
auto finder = [size](block_type const& lhs, block_type const& rhs) {
return lhs.is_better_fit(size, rhs);
};
auto const iter = std::min_element(cbegin(), cend(), finder);

if (iter != cend() && iter->fits(size)) {
// Remove the block from the free_list and return it.
Expand All @@ -248,6 +253,7 @@ struct coalescing_free_list : free_list<block> {
return block_type{}; // not found
}

#ifdef RMM_DEBUG_PRINT
/**
* @brief Print all blocks in the free_list.
*/
Expand All @@ -256,6 +262,7 @@ struct coalescing_free_list : free_list<block> {
std::cout << size() << '\n';
std::for_each(cbegin(), cend(), [](auto const iter) { iter.print(); });
}
#endif
}; // coalescing_free_list

} // namespace rmm::mr::detail
7 changes: 7 additions & 0 deletions include/rmm/mr/device/detail/free_list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,21 @@ struct block_base {
[[nodiscard]] inline void* pointer() const { return ptr; }
/// Returns true if this block is valid (non-null), false otherwise
[[nodiscard]] inline bool is_valid() const { return pointer() != nullptr; }

#ifdef RMM_DEBUG_PRINT
/// Prints the block to stdout
inline void print() const { std::cout << pointer(); }
#endif
};

#ifdef RMM_DEBUG_PRINT
/// Print block_base on an ostream
inline std::ostream& operator<<(std::ostream& out, const block_base& block)
{
out << block.pointer();
return out;
}
#endif

/**
* @brief Base class defining an interface for a list of free memory blocks.
Expand Down Expand Up @@ -115,6 +120,7 @@ class free_list {
*/
void clear() noexcept { blocks.clear(); }

#ifdef RMM_DEBUG_PRINT
/**
* @brief Print all blocks in the free_list.
*/
Expand All @@ -125,6 +131,7 @@ class free_list {
std::cout << block << std::endl;
}
}
#endif

protected:
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class stream_ordered_memory_resource : public crtp<PoolResource>, public device_
stream_free_blocks_[get_event(stream)].insert(std::move(blocks));
}

#ifdef RMM_DEBUG_PRINT
void print_free_blocks() const
{
std::cout << "stream free blocks: ";
Expand All @@ -170,6 +171,7 @@ class stream_ordered_memory_resource : public crtp<PoolResource>, public device_
}
std::cout << std::endl;
}
#endif

/**
* @brief Get the mutex object
Expand Down
8 changes: 5 additions & 3 deletions include/rmm/mr/device/fixed_size_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class fixed_size_memory_resource
*/
free_list blocks_from_upstream(cuda_stream_view stream)
{
void* ptr = upstream_mr_->allocate(upstream_chunk_size_, stream);
void* ptr = get_upstream()->allocate(upstream_chunk_size_, stream);
block_type block{ptr};
upstream_blocks_.push_back(block);

Expand Down Expand Up @@ -230,16 +230,17 @@ class fixed_size_memory_resource
lock_guard lock(this->get_mutex());

for (auto block : upstream_blocks_) {
upstream_mr_->deallocate(block.pointer(), upstream_chunk_size_);
get_upstream()->deallocate(block.pointer(), upstream_chunk_size_);
}
upstream_blocks_.clear();
}

#ifdef RMM_DEBUG_PRINT
void print()
{
lock_guard lock(this->get_mutex());

auto const [free, total] = upstream_mr_->get_mem_info(0);
auto const [free, total] = get_upstream()->get_mem_info(rmm::cuda_stream_default);
std::cout << "GPU free memory: " << free << " total: " << total << "\n";

std::cout << "upstream_blocks: " << upstream_blocks_.size() << "\n";
Expand All @@ -253,6 +254,7 @@ class fixed_size_memory_resource

this->print_free_blocks();
}
#endif

/**
* @brief Get the largest available block size and total free size in the specified free list
Expand Down
29 changes: 21 additions & 8 deletions include/rmm/mr/device/logging_resource_adaptor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <cstddef>
#include <memory>
#include <sstream>
#include <string_view>

namespace rmm::mr {
/**
Expand Down Expand Up @@ -67,10 +68,7 @@ class logging_resource_adaptor final : public device_memory_resource {
logging_resource_adaptor(Upstream* upstream,
std::string const& filename = get_default_filename(),
bool auto_flush = false)
: logger_{std::make_shared<spdlog::logger>(
"RMM",
std::make_shared<spdlog::sinks::basic_file_sink_mt>(filename, true /*truncate file*/))},
upstream_{upstream}
: logger_{make_logger(filename)}, upstream_{upstream}
{
RMM_EXPECTS(nullptr != upstream, "Unexpected null upstream resource pointer.");

Expand All @@ -92,9 +90,7 @@ class logging_resource_adaptor final : public device_memory_resource {
* performance.
*/
logging_resource_adaptor(Upstream* upstream, std::ostream& stream, bool auto_flush = false)
: logger_{std::make_shared<spdlog::logger>(
"RMM", std::make_shared<spdlog::sinks::ostream_sink_mt>(stream))},
upstream_{upstream}
: logger_{make_logger(stream)}, upstream_{upstream}
{
RMM_EXPECTS(nullptr != upstream, "Unexpected null upstream resource pointer.");

Expand All @@ -104,7 +100,7 @@ class logging_resource_adaptor final : public device_memory_resource {
logging_resource_adaptor(Upstream* upstream,
spdlog::sinks_init_list sinks,
bool auto_flush = false)
: logger_{std::make_shared<spdlog::logger>("RMM", sinks)}, upstream_{upstream}
: logger_{make_logger(sinks)}, upstream_{upstream}
{
RMM_EXPECTS(nullptr != upstream, "Unexpected null upstream resource pointer.");

Expand Down Expand Up @@ -177,6 +173,23 @@ class logging_resource_adaptor final : public device_memory_resource {
return std::string{filename};
}

static auto make_logger(std::ostream& stream)
{
return std::make_shared<spdlog::logger>(
"RMM", std::make_shared<spdlog::sinks::ostream_sink_mt>(stream));
}

static auto make_logger(std::string const& filename)
{
return std::make_shared<spdlog::logger>(
"RMM", std::make_shared<spdlog::sinks::basic_file_sink_mt>(filename, true /*truncate file*/));
}

static auto make_logger(spdlog::sinks_init_list sinks)
{
return std::make_shared<spdlog::logger>("RMM", sinks);
}

/**
* @brief Initialize the logger.
*/
Expand Down
Loading