Improve C++ Test Coverage (#920)

Now that #905 added C++ code coverage support, this PR improves test coverage. Much of the improvement comes from adding a new `adaptor_test.cpp` which generically tests the common functions of all 7 adaptor types. Small test additions improve coverage of many other files as well. At least one typo bug was uncovered and fixed.

If you build with `-DCODE_COVERAGE=ON -DRMM_LOGGING_LEVEL=TRACE`, overall code coverage is now 99.5% and all but one file has 100% coverage. That one file, arena.hpp is undergoing work concurrent to this PR, and improvement to 100% requires additional testing that might be best undertaken by @rongou, so I will leave it.

![image](https://user-images.githubusercontent.com/783069/143160783-aecda008-2d7d-4ceb-8982-34bb1a35174c.png)

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Rong Ou (https://github.com/rongou)
  - Jake Hemstad (https://github.com/jrhemstad)

URL: #920

gcovr.cfg

@@ -1,4 +1,7 @@
 exclude=build/.*
 exclude=tests/.*
+exclude=benchmarks/.*
 html=yes
 html-details=yes
+sort-percentage=yes
+exclude-throw-branches=yes

include/rmm/detail/error.hpp

@@ -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(); }
 
@@ -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()} {}
 };
 
 /**

include/rmm/detail/stack_trace.hpp

@@ -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__))
@@ -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;

include/rmm/logger.hpp

@@ -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) {

include/rmm/mr/device/arena_memory_resource.hpp

@@ -280,6 +280,7 @@ class arena_memory_resource final : public device_memory_resource {
         stream_arena.second.dump_memory_log(logger_);
       }
     }
+    logger_->flush();
   }
 
   /**

include/rmm/mr/device/detail/arena.hpp

@@ -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.

include/rmm/mr/device/detail/coalescing_free_list.hpp

@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <iterator>
 #include <rmm/detail/error.hpp>
 #include <rmm/mr/device/detail/free_list.hpp>
 
@@ -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.
@@ -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);
   }
 
   /**
@@ -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.
@@ -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.
    */
@@ -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

include/rmm/mr/device/detail/free_list.hpp

@@ -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.
@@ -115,6 +120,7 @@ class free_list {
    */
   void clear() noexcept { blocks.clear(); }
 
+#ifdef RMM_DEBUG_PRINT
   /**
    * @brief Print all blocks in the free_list.
    */
@@ -125,6 +131,7 @@ class free_list {
       std::cout << block << std::endl;
     }
   }
+#endif
 
  protected:
   /**

include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp

@@ -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: ";
@@ -170,6 +171,7 @@ class stream_ordered_memory_resource : public crtp<PoolResource>, public device_
     }
     std::cout << std::endl;
   }
+#endif
 
   /**
    * @brief Get the mutex object

include/rmm/mr/device/fixed_size_memory_resource.hpp

@@ -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);
 
@@ -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";
@@ -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

include/rmm/mr/device/logging_resource_adaptor.hpp

@@ -27,6 +27,7 @@
 #include <cstddef>
 #include <memory>
 #include <sstream>
+#include <string_view>
 
 namespace rmm::mr {
 /**
@@ -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.");
 
@@ -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.");
 
@@ -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.");
 
@@ -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.
    */