From 6066861d7b06d41ed926c176b8e278b19073027b Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 7 Nov 2024 05:46:52 +0000 Subject: [PATCH 01/21] Remove all explicit usage of fmtlib --- include/rmm/detail/format.hpp | 57 +++++++++++++++++++ include/rmm/logger.hpp | 36 +----------- .../rmm/mr/device/arena_memory_resource.hpp | 3 +- include/rmm/mr/device/detail/arena.hpp | 20 +++---- .../mr/device/detail/coalescing_free_list.hpp | 9 +-- .../detail/stream_ordered_memory_resource.hpp | 27 +++++---- .../mr/device/logging_resource_adaptor.hpp | 10 ++-- .../rmm/mr/device/pool_memory_resource.hpp | 7 +-- .../mr/device/tracking_resource_adaptor.hpp | 6 +- tests/logger_tests.cpp | 2 +- tests/mr/device/callback_mr_tests.cpp | 7 ++- 11 files changed, 101 insertions(+), 83 deletions(-) create mode 100644 include/rmm/detail/format.hpp diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp new file mode 100644 index 000000000..1fdc49b0a --- /dev/null +++ b/include/rmm/detail/format.hpp @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2024, 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 + +#include +#include + +namespace RMM_NAMESPACE { +namespace detail { + +// Stringify a size in bytes to a human-readable value +inline std::string format_bytes(std::size_t value) +{ + static std::array units{"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"}; + + int index = 0; + auto size = static_cast(value); + while (size > 1024) { + size /= 1024; + index++; + } + try { + return std::to_string(value) + ' ' + units.at(index); + } catch (std::out_of_range const& e) { + return {std::string{"value out of range: "} + e.what()}; + } +} + +// Stringify a stream ID +inline std::string format_stream(rmm::cuda_stream_view stream) +{ + try { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + return std::to_string(reinterpret_cast(stream.value())); + } catch (std::exception const& e) { + return {std::string{"Exception: "} + e.what()}; + } +} + +} // namespace detail +} // namespace RMM_NAMESPACE \ No newline at end of file diff --git a/include/rmm/logger.hpp b/include/rmm/logger.hpp index eba3f122b..d955d7c5e 100644 --- a/include/rmm/logger.hpp +++ b/include/rmm/logger.hpp @@ -16,15 +16,12 @@ #pragma once +#include #include -#include -#include #include #include -#include -#include #include namespace RMM_NAMESPACE { @@ -70,32 +67,6 @@ struct logger_wrapper { } }; -/** - * @brief Represent a size in number of bytes. - */ -struct bytes { - std::size_t value; ///< The size in bytes - - /** - * @brief Construct a new bytes object - * - * @param os The output stream - * @param value The size in bytes - */ - friend std::ostream& operator<<(std::ostream& os, bytes const& value) - { - static std::array units{"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"}; - - int index = 0; - auto size = static_cast(value.value); - while (size > 1024) { - size /= 1024; - index++; - } - return os << size << ' ' << units.at(index); - } -}; - inline spdlog::logger& logger() { static detail::logger_wrapper wrapped{}; @@ -136,9 +107,4 @@ logger() } // namespace RMM_NAMESPACE -// Doxygen doesn't like this because we're overloading something from fmt -//! @cond Doxygen_Suppress -template <> -struct fmt::formatter : fmt::ostream_formatter {}; - //! @endcond diff --git a/include/rmm/mr/device/arena_memory_resource.hpp b/include/rmm/mr/device/arena_memory_resource.hpp index 9b380ffb9..cac5b2530 100644 --- a/include/rmm/mr/device/arena_memory_resource.hpp +++ b/include/rmm/mr/device/arena_memory_resource.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -335,7 +336,7 @@ class arena_memory_resource final : public device_memory_resource { void dump_memory_log(size_t bytes) { logger_->info("**************************************************"); - logger_->info("Ran out of memory trying to allocate {}.", rmm::detail::bytes{bytes}); + logger_->info("Ran out of memory trying to allocate {}.", rmm::detail::format_bytes(bytes)); logger_->info("**************************************************"); logger_->info("Global arena:"); global_arena_.dump_memory_log(logger_); diff --git a/include/rmm/mr/device/detail/arena.hpp b/include/rmm/mr/device/detail/arena.hpp index da64ca85b..17bace387 100644 --- a/include/rmm/mr/device/detail/arena.hpp +++ b/include/rmm/mr/device/detail/arena.hpp @@ -21,13 +21,13 @@ #include #include #include +#include #include #include #include #include -#include #include #include @@ -651,16 +651,16 @@ class global_arena final { { std::lock_guard lock(mtx_); - logger->info(" Arena size: {}", rmm::detail::bytes{upstream_block_.size()}); + logger->info(" Arena size: {}", rmm::detail::format_bytes(upstream_block_.size())); logger->info(" # superblocks: {}", superblocks_.size()); if (!superblocks_.empty()) { logger->debug(" Total size of superblocks: {}", - rmm::detail::bytes{total_memory_size(superblocks_)}); + rmm::detail::format_bytes(total_memory_size(superblocks_))); auto const total_free = total_free_size(superblocks_); auto const max_free = max_free_size(superblocks_); auto const fragmentation = (1 - max_free / static_cast(total_free)) * 100; - logger->info(" Total free memory: {}", rmm::detail::bytes{total_free}); - logger->info(" Largest block of free memory: {}", rmm::detail::bytes{max_free}); + logger->info(" Total free memory: {}", rmm::detail::format_bytes(total_free)); + logger->info(" Largest block of free memory: {}", rmm::detail::format_bytes(max_free)); logger->info(" Fragmentation: {:.2f}%", fragmentation); auto index = 0; @@ -671,13 +671,13 @@ class global_arena final { " Superblock {}: start={}, end={}, size={}, empty={}, # free blocks={}, max free={}, " "gap={}", index, - fmt::ptr(sblk.pointer()), - fmt::ptr(sblk.end()), - rmm::detail::bytes{sblk.size()}, + sblk.pointer(), + sblk.end(), + rmm::detail::format_bytes(sblk.size()), sblk.empty(), sblk.free_blocks(), - rmm::detail::bytes{sblk.max_free_size()}, - rmm::detail::bytes{static_cast(sblk.pointer() - prev_end)}); + rmm::detail::format_bytes(sblk.max_free_size()), + rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end))); prev_end = sblk.end(); index++; } diff --git a/include/rmm/mr/device/detail/coalescing_free_list.hpp b/include/rmm/mr/device/detail/coalescing_free_list.hpp index 8d5cbf9ed..8b056e6d9 100644 --- a/include/rmm/mr/device/detail/coalescing_free_list.hpp +++ b/include/rmm/mr/device/detail/coalescing_free_list.hpp @@ -20,8 +20,6 @@ #include #include -#include - #include #include #include @@ -131,10 +129,7 @@ struct block : public block_base { /** * @brief Print this block. For debugging. */ - inline void print() const - { - std::cout << fmt::format("{} {} B", fmt::ptr(pointer()), size()) << std::endl; - } + inline void print() const { std::cout << pointer() << " " << size() << " B" << std::endl; } #endif private: @@ -146,7 +141,7 @@ struct block : public block_base { /// 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()); + out << blk.pointer() << " " << blk.size() << " B" << std::endl; return out; } #endif diff --git a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp index 9cf674d6e..b3de01dba 100644 --- a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp +++ b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp @@ -19,13 +19,12 @@ #include #include #include +#include #include #include #include -#include - #include #include #include @@ -201,7 +200,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void* do_allocate(std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[A][stream {:p}][{}B]", fmt::ptr(stream.value()), size); + RMM_LOG_TRACE("[A][stream {}][{}B]", rmm::detail::format_stream(stream), size); if (size <= 0) { return nullptr; } @@ -215,10 +214,10 @@ class stream_ordered_memory_resource : public crtp, public device_ rmm::out_of_memory); auto const block = this->underlying().get_block(size, stream_event); - RMM_LOG_TRACE("[A][stream {:p}][{}B][{:p}]", - fmt::ptr(stream_event.stream), + RMM_LOG_TRACE("[A][stream {}][{}B][{}]", + rmm::detail::format_stream(stream_event.stream), size, - fmt::ptr(block.pointer())); + block.pointer()); log_summary_trace(); @@ -234,7 +233,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void do_deallocate(void* ptr, std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[D][stream {:p}][{}B][{:p}]", fmt::ptr(stream.value()), size, ptr); + RMM_LOG_TRACE("[D][stream {}][{}B][{}]", rmm::detail::format_stream(stream), size, ptr); if (size <= 0 || ptr == nullptr) { return; } @@ -384,10 +383,10 @@ class stream_ordered_memory_resource : public crtp, public device_ if (merge_first) { merge_lists(stream_event, blocks, other_event, std::move(other_blocks)); - RMM_LOG_DEBUG("[A][Stream {:p}][{}B][Merged stream {:p}]", - fmt::ptr(stream_event.stream), + RMM_LOG_DEBUG("[A][Stream {}][{}B][Merged stream {}]", + rmm::detail::format_stream(stream_event.stream), size, - fmt::ptr(iter->first.stream)); + rmm::detail::format_stream(iter->first.stream)); stream_free_blocks_.erase(iter); @@ -414,11 +413,11 @@ class stream_ordered_memory_resource : public crtp, public device_ block_type const block = find_block(iter); if (block.is_valid()) { - RMM_LOG_DEBUG((merge_first) ? "[A][Stream {:p}][{}B][Found after merging stream {:p}]" - : "[A][Stream {:p}][{}B][Taken from stream {:p}]", - fmt::ptr(stream_event.stream), + RMM_LOG_DEBUG((merge_first) ? "[A][Stream {}][{}B][Found after merging stream {}]" + : "[A][Stream {}][{}B][Taken from stream {}]", + rmm::detail::format_stream(stream_event.stream), size, - fmt::ptr(iter->first.stream)); + rmm::detail::format_stream(iter->first.stream)); return block; } } diff --git a/include/rmm/mr/device/logging_resource_adaptor.hpp b/include/rmm/mr/device/logging_resource_adaptor.hpp index 595ab2e4e..13dab78dd 100644 --- a/include/rmm/mr/device/logging_resource_adaptor.hpp +++ b/include/rmm/mr/device/logging_resource_adaptor.hpp @@ -18,16 +18,17 @@ #include #include #include +#include #include #include -#include #include #include #include #include #include +#include #include #include #include @@ -297,10 +298,11 @@ class logging_resource_adaptor final : public device_memory_resource { { try { auto const ptr = get_upstream_resource().allocate_async(bytes, stream); - logger_->info("allocate,{},{},{}", ptr, bytes, fmt::ptr(stream.value())); + logger_->info("allocate,{},{},{}", ptr, bytes, rmm::detail::format_stream(stream)); return ptr; } catch (...) { - logger_->info("allocate failure,{},{},{}", nullptr, bytes, fmt::ptr(stream.value())); + logger_->info( + "allocate failure,{},{},{}", nullptr, bytes, rmm::detail::format_stream(stream)); throw; } } @@ -321,7 +323,7 @@ class logging_resource_adaptor final : public device_memory_resource { */ void do_deallocate(void* ptr, std::size_t bytes, cuda_stream_view stream) override { - logger_->info("free,{},{},{}", ptr, bytes, fmt::ptr(stream.value())); + logger_->info("free,{},{},{}", ptr, bytes, rmm::detail::format_stream(stream)); get_upstream_resource().deallocate_async(ptr, bytes, stream); } diff --git a/include/rmm/mr/device/pool_memory_resource.hpp b/include/rmm/mr/device/pool_memory_resource.hpp index f63de21ff..2b13922ec 100644 --- a/include/rmm/mr/device/pool_memory_resource.hpp +++ b/include/rmm/mr/device/pool_memory_resource.hpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -34,8 +35,6 @@ #include #include -#include - #include #include #include @@ -272,7 +271,7 @@ class pool_memory_resource final try_size = std::max(min_size, try_size / 2); } RMM_LOG_ERROR("[A][Stream {}][Upstream {}B][FAILURE maximum pool size exceeded]", - fmt::ptr(stream.value()), + rmm::detail::format_stream(stream), min_size); RMM_FAIL("Maximum pool size exceeded", rmm::out_of_memory); } @@ -351,7 +350,7 @@ class pool_memory_resource final */ std::optional block_from_upstream(std::size_t size, cuda_stream_view stream) { - RMM_LOG_DEBUG("[A][Stream {}][Upstream {}B]", fmt::ptr(stream.value()), size); + RMM_LOG_DEBUG("[A][Stream {}][Upstream {}B]", rmm::detail::format_stream(stream), size); if (size == 0) { return {}; } diff --git a/include/rmm/mr/device/tracking_resource_adaptor.hpp b/include/rmm/mr/device/tracking_resource_adaptor.hpp index 6a5916e5c..7457ae748 100644 --- a/include/rmm/mr/device/tracking_resource_adaptor.hpp +++ b/include/rmm/mr/device/tracking_resource_adaptor.hpp @@ -23,8 +23,6 @@ #include #include -#include - #include #include #include @@ -239,9 +237,9 @@ class tracking_resource_adaptor final : public device_memory_resource { // 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: " + "Deallocating a pointer that was not tracked. Ptr: {} [{}B], Current Num. Allocations: " "{}", - fmt::ptr(ptr), + ptr, bytes, this->allocations_.size()); } else { diff --git a/tests/logger_tests.cpp b/tests/logger_tests.cpp index 643281d91..7148c2d55 100644 --- a/tests/logger_tests.cpp +++ b/tests/logger_tests.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -112,7 +113,6 @@ void expect_log_events(std::string const& filename, // EXPECT_EQ(expected.thread_id, actual.thread_id); // EXPECT_EQ(expected.stream, actual.stream); EXPECT_EQ(expected.act, actual.act); - // device_memory_resource automatically pads an allocation to a multiple of 8 bytes EXPECT_EQ(expected.size, actual.size); EXPECT_EQ(expected.pointer, actual.pointer); return true; diff --git a/tests/mr/device/callback_mr_tests.cpp b/tests/mr/device/callback_mr_tests.cpp index a56efa60c..a7f6ab7be 100644 --- a/tests/mr/device/callback_mr_tests.cpp +++ b/tests/mr/device/callback_mr_tests.cpp @@ -23,11 +23,11 @@ #include #include -#include #include #include #include +#include namespace rmm::test { namespace { @@ -78,8 +78,9 @@ TEST(CallbackTest, LoggingTest) auto* ptr = mr.allocate(size); mr.deallocate(ptr, size); - std::string output = testing::internal::GetCapturedStdout(); - std::string expect = fmt::format("Allocating {} bytes\nDeallocating {} bytes\n", size, size); + auto output = testing::internal::GetCapturedStdout(); + auto expect = std::string("Allocating ") + std::to_string(size) + " bytes\nDeallocating " + + std::to_string(size) + " bytes\n"; ASSERT_EQ(expect, output); } From fbbb1b57c1d726ee03a431b96b002f11fffa94e1 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 7 Nov 2024 05:56:41 +0000 Subject: [PATCH 02/21] Use detail::logger() in RMM_LOGGING_ASSERT to eliminate deprecation warnings in debug build. --- include/rmm/detail/logging_assert.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rmm/detail/logging_assert.hpp b/include/rmm/detail/logging_assert.hpp index 7eb667211..4f8e31b8d 100644 --- a/include/rmm/detail/logging_assert.hpp +++ b/include/rmm/detail/logging_assert.hpp @@ -38,7 +38,7 @@ if (!success) { \ RMM_LOG_CRITICAL( \ "[" __FILE__ ":" RMM_STRINGIFY(__LINE__) "] Assertion " RMM_STRINGIFY(_expr) " failed."); \ - rmm::logger().flush(); \ + rmm::detail::logger().flush(); \ /* NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) */ \ assert(success); \ } \ From 966f706fff9e71690ef170b230d08fd5cba4e2ce Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 7 Nov 2024 05:56:59 +0000 Subject: [PATCH 03/21] Remove temporary try/catch from debugging. --- include/rmm/detail/format.hpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index 1fdc49b0a..7a2eba883 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -35,22 +35,15 @@ inline std::string format_bytes(std::size_t value) size /= 1024; index++; } - try { - return std::to_string(value) + ' ' + units.at(index); - } catch (std::out_of_range const& e) { - return {std::string{"value out of range: "} + e.what()}; - } + + return std::to_string(value) + ' ' + units.at(index); } // Stringify a stream ID inline std::string format_stream(rmm::cuda_stream_view stream) { - try { - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - return std::to_string(reinterpret_cast(stream.value())); - } catch (std::exception const& e) { - return {std::string{"Exception: "} + e.what()}; - } + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + return std::to_string(reinterpret_cast(stream.value())); } } // namespace detail From 6e31e4733049dd58a13ef865bd6d332b1b95cd9d Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 7 Nov 2024 05:57:51 +0000 Subject: [PATCH 04/21] \n @ EOF --- include/rmm/detail/format.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index 7a2eba883..65072cd5b 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -47,4 +47,4 @@ inline std::string format_stream(rmm::cuda_stream_view stream) } } // namespace detail -} // namespace RMM_NAMESPACE \ No newline at end of file +} // namespace RMM_NAMESPACE From f00beb7a324aa09e5541a21c7a95cccf1bdd35d9 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 7 Nov 2024 05:59:28 +0000 Subject: [PATCH 05/21] copyright --- include/rmm/detail/logging_assert.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rmm/detail/logging_assert.hpp b/include/rmm/detail/logging_assert.hpp index 4f8e31b8d..4d702ee2b 100644 --- a/include/rmm/detail/logging_assert.hpp +++ b/include/rmm/detail/logging_assert.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, NVIDIA CORPORATION. + * Copyright (c) 2020-2024, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 92f179f2e4e730b9ff7fd07ce8db1f482751c5b0 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Mon, 11 Nov 2024 02:34:50 +0000 Subject: [PATCH 06/21] Added `formatted_log` utility and switch to printf-style placeholders everywhere. --- benchmarks/utilities/log_parser.hpp | 2 +- include/rmm/detail/format.hpp | 25 ++++++++++++++++++ include/rmm/logger.hpp | 19 +++++++++----- .../rmm/mr/device/arena_memory_resource.hpp | 3 ++- include/rmm/mr/device/detail/arena.hpp | 26 +++++++++++-------- .../detail/stream_ordered_memory_resource.hpp | 14 +++++----- .../mr/device/logging_resource_adaptor.hpp | 9 ++++--- .../rmm/mr/device/pool_memory_resource.hpp | 4 +-- .../mr/device/tracking_resource_adaptor.hpp | 8 +++--- 9 files changed, 74 insertions(+), 36 deletions(-) diff --git a/benchmarks/utilities/log_parser.hpp b/benchmarks/utilities/log_parser.hpp index 2283ace93..b48ece392 100644 --- a/benchmarks/utilities/log_parser.hpp +++ b/benchmarks/utilities/log_parser.hpp @@ -151,7 +151,7 @@ inline std::vector parse_csv(std::string const& filename) auto parse_pointer = [](std::string const& str, uintptr_t& ptr) { auto const base{16}; - ptr = std::stoll(str, nullptr, base); + ptr = (str == "(nil)") ? 0 : std::stoll(str, nullptr, base); }; std::vector pointers = csv.GetColumn("Pointer", parse_pointer); diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index 65072cd5b..ef78e68d0 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -19,11 +19,36 @@ #include #include +#include +#include +#include #include namespace RMM_NAMESPACE { namespace detail { +/** + * @brief Format a message string with printf-style formatting + * + * This function performs printf-style formatting to avoid the need for fmt + * or spdlog's own templated APIs (which would require exposing spdlog + * symbols publicly) and returns the formatted message as a `std::string`. + * + * @param format The format string + * @param args The format arguments + */ +template +std::string formatted_log(std::string const& format, Args&&... args) +{ + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) + auto size = static_cast(std::snprintf(nullptr, 0, format.c_str(), args...) + 1); + if (size <= 0) { throw std::runtime_error("Error during formatting."); } + std::unique_ptr buf(new char[size]); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) + std::snprintf(buf.get(), size, format.c_str(), args...); + return {buf.get(), buf.get() + size - 1}; // drop '\0' +} + // Stringify a size in bytes to a human-readable value inline std::string format_bytes(std::size_t value) { diff --git a/include/rmm/logger.hpp b/include/rmm/logger.hpp index d955d7c5e..2cfd921b1 100644 --- a/include/rmm/logger.hpp +++ b/include/rmm/logger.hpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -96,12 +97,18 @@ logger() // The default is INFO, but it should be used sparingly, so that by default a log file is only // output if there is important information, warnings, errors, and critical failures // Log messages that require computation should only be used at level TRACE and DEBUG -#define RMM_LOG_TRACE(...) SPDLOG_LOGGER_TRACE(&rmm::detail::logger(), __VA_ARGS__) -#define RMM_LOG_DEBUG(...) SPDLOG_LOGGER_DEBUG(&rmm::detail::logger(), __VA_ARGS__) -#define RMM_LOG_INFO(...) SPDLOG_LOGGER_INFO(&rmm::detail::logger(), __VA_ARGS__) -#define RMM_LOG_WARN(...) SPDLOG_LOGGER_WARN(&rmm::detail::logger(), __VA_ARGS__) -#define RMM_LOG_ERROR(...) SPDLOG_LOGGER_ERROR(&rmm::detail::logger(), __VA_ARGS__) -#define RMM_LOG_CRITICAL(...) SPDLOG_LOGGER_CRITICAL(&rmm::detail::logger(), __VA_ARGS__) +#define RMM_LOG_TRACE(...) \ + SPDLOG_LOGGER_TRACE(&rmm::detail::logger(), rmm::detail::formatted_log(__VA_ARGS__)) +#define RMM_LOG_DEBUG(...) \ + SPDLOG_LOGGER_DEBUG(&rmm::detail::logger(), rmm::detail::formatted_log(__VA_ARGS__)) +#define RMM_LOG_INFO(...) \ + SPDLOG_LOGGER_INFO(&rmm::detail::logger(), rmm::detail::formatted_log(__VA_ARGS__)) +#define RMM_LOG_WARN(...) \ + SPDLOG_LOGGER_WARN(&rmm::detail::logger(), rmm::detail::formatted_log(__VA_ARGS__)) +#define RMM_LOG_ERROR(...) \ + SPDLOG_LOGGER_ERROR(&rmm::detail::logger(), rmm::detail::formatted_log(__VA_ARGS__)) +#define RMM_LOG_CRITICAL(...) \ + SPDLOG_LOGGER_CRITICAL(&rmm::detail::logger(), rmm::detail::formatted_log(__VA_ARGS__)) //! @endcond diff --git a/include/rmm/mr/device/arena_memory_resource.hpp b/include/rmm/mr/device/arena_memory_resource.hpp index cac5b2530..d3a4bb09d 100644 --- a/include/rmm/mr/device/arena_memory_resource.hpp +++ b/include/rmm/mr/device/arena_memory_resource.hpp @@ -336,7 +336,8 @@ class arena_memory_resource final : public device_memory_resource { void dump_memory_log(size_t bytes) { logger_->info("**************************************************"); - logger_->info("Ran out of memory trying to allocate {}.", rmm::detail::format_bytes(bytes)); + logger_->info(rmm::detail::formatted_log("Ran out of memory trying to allocate %s.", + rmm::detail::format_bytes(bytes))); logger_->info("**************************************************"); logger_->info("Global arena:"); global_arena_.dump_memory_log(logger_); diff --git a/include/rmm/mr/device/detail/arena.hpp b/include/rmm/mr/device/detail/arena.hpp index 17bace387..23684a490 100644 --- a/include/rmm/mr/device/detail/arena.hpp +++ b/include/rmm/mr/device/detail/arena.hpp @@ -651,25 +651,29 @@ class global_arena final { { std::lock_guard lock(mtx_); - logger->info(" Arena size: {}", rmm::detail::format_bytes(upstream_block_.size())); - logger->info(" # superblocks: {}", superblocks_.size()); + logger->info(rmm::detail::formatted_log(" Arena size: %s", + rmm::detail::format_bytes(upstream_block_.size()))); + logger->info(rmm::detail::formatted_log(" # superblocks: %d", superblocks_.size())); if (!superblocks_.empty()) { - logger->debug(" Total size of superblocks: {}", - rmm::detail::format_bytes(total_memory_size(superblocks_))); + logger->debug( + rmm::detail::formatted_log(" Total size of superblocks: %s", + rmm::detail::format_bytes(total_memory_size(superblocks_)))); auto const total_free = total_free_size(superblocks_); auto const max_free = max_free_size(superblocks_); auto const fragmentation = (1 - max_free / static_cast(total_free)) * 100; - logger->info(" Total free memory: {}", rmm::detail::format_bytes(total_free)); - logger->info(" Largest block of free memory: {}", rmm::detail::format_bytes(max_free)); - logger->info(" Fragmentation: {:.2f}%", fragmentation); + logger->info(rmm::detail::formatted_log(" Total free memory: %s", + rmm::detail::format_bytes(total_free))); + logger->info(rmm::detail::formatted_log(" Largest block of free memory: %s", + rmm::detail::format_bytes(max_free))); + logger->info(rmm::detail::formatted_log(" Fragmentation: %0.2s", fragmentation)); auto index = 0; char* prev_end{}; for (auto const& sblk : superblocks_) { if (prev_end == nullptr) { prev_end = sblk.pointer(); } - logger->debug( - " Superblock {}: start={}, end={}, size={}, empty={}, # free blocks={}, max free={}, " - "gap={}", + logger->debug(rmm::detail::formatted_log( + " Superblock %d: start=%p, end=%p, size=%d, empty=%d, # free blocks=%d, max free=%s, " + "gap=%s", index, sblk.pointer(), sblk.end(), @@ -677,7 +681,7 @@ class global_arena final { sblk.empty(), sblk.free_blocks(), rmm::detail::format_bytes(sblk.max_free_size()), - rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end))); + rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end)))); prev_end = sblk.end(); index++; } diff --git a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp index b3de01dba..519da2295 100644 --- a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp +++ b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp @@ -200,7 +200,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void* do_allocate(std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[A][stream {}][{}B]", rmm::detail::format_stream(stream), size); + RMM_LOG_TRACE("[A][stream %s][%dB]", rmm::detail::format_stream(stream), size); if (size <= 0) { return nullptr; } @@ -214,7 +214,7 @@ class stream_ordered_memory_resource : public crtp, public device_ rmm::out_of_memory); auto const block = this->underlying().get_block(size, stream_event); - RMM_LOG_TRACE("[A][stream {}][{}B][{}]", + RMM_LOG_TRACE("[A][stream %s][%dB][%p]", rmm::detail::format_stream(stream_event.stream), size, block.pointer()); @@ -233,7 +233,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void do_deallocate(void* ptr, std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[D][stream {}][{}B][{}]", rmm::detail::format_stream(stream), size, ptr); + RMM_LOG_TRACE("[D][stream %s][%dB][%p]", rmm::detail::format_stream(stream), size, ptr); if (size <= 0 || ptr == nullptr) { return; } @@ -383,7 +383,7 @@ class stream_ordered_memory_resource : public crtp, public device_ if (merge_first) { merge_lists(stream_event, blocks, other_event, std::move(other_blocks)); - RMM_LOG_DEBUG("[A][Stream {}][{}B][Merged stream {}]", + RMM_LOG_DEBUG("[A][Stream %s][%dB][Merged stream %s]", rmm::detail::format_stream(stream_event.stream), size, rmm::detail::format_stream(iter->first.stream)); @@ -413,8 +413,8 @@ class stream_ordered_memory_resource : public crtp, public device_ block_type const block = find_block(iter); if (block.is_valid()) { - RMM_LOG_DEBUG((merge_first) ? "[A][Stream {}][{}B][Found after merging stream {}]" - : "[A][Stream {}][{}B][Taken from stream {}]", + RMM_LOG_DEBUG((merge_first) ? "[A][Stream %s][%dB][Found after merging stream %s]" + : "[A][Stream %s][%dB][Taken from stream %s]", rmm::detail::format_stream(stream_event.stream), size, rmm::detail::format_stream(iter->first.stream)); @@ -470,7 +470,7 @@ class stream_ordered_memory_resource : public crtp, public device_ max_block = std::max(summary.first, max_block); free_mem += summary.second; }); - RMM_LOG_TRACE("[Summary][Free lists: {}][Blocks: {}][Max Block: {}][Total Free: {}]", + RMM_LOG_TRACE("[Summary][Free lists: %d][Blocks: %d][Max Block: %d][Total Free: %d]", stream_free_blocks_.size(), num_blocks, max_block, diff --git a/include/rmm/mr/device/logging_resource_adaptor.hpp b/include/rmm/mr/device/logging_resource_adaptor.hpp index 13dab78dd..61b41d408 100644 --- a/include/rmm/mr/device/logging_resource_adaptor.hpp +++ b/include/rmm/mr/device/logging_resource_adaptor.hpp @@ -298,11 +298,12 @@ class logging_resource_adaptor final : public device_memory_resource { { try { auto const ptr = get_upstream_resource().allocate_async(bytes, stream); - logger_->info("allocate,{},{},{}", ptr, bytes, rmm::detail::format_stream(stream)); + logger_->info( + rmm::detail::formatted_log("allocate,%p,%lld,%lld", ptr, bytes, stream.value())); return ptr; } catch (...) { - logger_->info( - "allocate failure,{},{},{}", nullptr, bytes, rmm::detail::format_stream(stream)); + logger_->info(rmm::detail::formatted_log( + "allocate failure,%p,%lld,%lld", nullptr, bytes, stream.value())); throw; } } @@ -323,7 +324,7 @@ class logging_resource_adaptor final : public device_memory_resource { */ void do_deallocate(void* ptr, std::size_t bytes, cuda_stream_view stream) override { - logger_->info("free,{},{},{}", ptr, bytes, rmm::detail::format_stream(stream)); + logger_->info(rmm::detail::formatted_log("free,%p,%d,%d", ptr, bytes, stream.value())); get_upstream_resource().deallocate_async(ptr, bytes, stream); } diff --git a/include/rmm/mr/device/pool_memory_resource.hpp b/include/rmm/mr/device/pool_memory_resource.hpp index 2b13922ec..4fff5a54c 100644 --- a/include/rmm/mr/device/pool_memory_resource.hpp +++ b/include/rmm/mr/device/pool_memory_resource.hpp @@ -270,7 +270,7 @@ class pool_memory_resource final } try_size = std::max(min_size, try_size / 2); } - RMM_LOG_ERROR("[A][Stream {}][Upstream {}B][FAILURE maximum pool size exceeded]", + RMM_LOG_ERROR("[A][Stream %s][Upstream %dB][FAILURE maximum pool size exceeded]", rmm::detail::format_stream(stream), min_size); RMM_FAIL("Maximum pool size exceeded", rmm::out_of_memory); @@ -350,7 +350,7 @@ class pool_memory_resource final */ std::optional block_from_upstream(std::size_t size, cuda_stream_view stream) { - RMM_LOG_DEBUG("[A][Stream {}][Upstream {}B]", rmm::detail::format_stream(stream), size); + RMM_LOG_DEBUG("[A][Stream %s][Upstream %dB]", rmm::detail::format_stream(stream), size); if (size == 0) { return {}; } diff --git a/include/rmm/mr/device/tracking_resource_adaptor.hpp b/include/rmm/mr/device/tracking_resource_adaptor.hpp index 7457ae748..1c42e4222 100644 --- a/include/rmm/mr/device/tracking_resource_adaptor.hpp +++ b/include/rmm/mr/device/tracking_resource_adaptor.hpp @@ -186,7 +186,7 @@ class tracking_resource_adaptor final : public device_memory_resource { void log_outstanding_allocations() const { #if SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG - RMM_LOG_DEBUG("Outstanding Allocations: {}", get_outstanding_allocations_str()); + RMM_LOG_DEBUG("Outstanding Allocations: %d", get_outstanding_allocations_str()); #endif // SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG } @@ -237,8 +237,8 @@ class tracking_resource_adaptor final : public device_memory_resource { // 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: {} [{}B], Current Num. Allocations: " - "{}", + "Deallocating a pointer that was not tracked. Ptr: %p [$dB], Current Num. Allocations: " + "%d", ptr, bytes, this->allocations_.size()); @@ -251,7 +251,7 @@ class tracking_resource_adaptor final : public device_memory_resource { // Don't throw but log an error. Throwing in a descructor (or any noexcept) will call // std::terminate RMM_LOG_ERROR( - "Alloc bytes ({}) and Dealloc bytes ({}) do not match", allocated_bytes, bytes); + "Alloc bytes (%d) and Dealloc bytes (%d) do not match", allocated_bytes, bytes); bytes = allocated_bytes; } From cfcdf4cae37c04acb6fa917d1b91f6c888d1eaf0 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Mon, 11 Nov 2024 02:40:40 +0000 Subject: [PATCH 07/21] Copyright --- benchmarks/utilities/log_parser.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/utilities/log_parser.hpp b/benchmarks/utilities/log_parser.hpp index b48ece392..4dfa5bae4 100644 --- a/benchmarks/utilities/log_parser.hpp +++ b/benchmarks/utilities/log_parser.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2021, NVIDIA CORPORATION. + * Copyright (c) 2020-2024, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 708ffba034fcf9b10c885ab0a2908d14db4ba814 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Tue, 12 Nov 2024 22:56:11 +0000 Subject: [PATCH 08/21] Improve typing and docs in formatted_log --- include/rmm/detail/format.hpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index ef78e68d0..47fe77df4 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -36,13 +36,17 @@ namespace detail { * * @param format The format string * @param args The format arguments + * @return The formatted message + * @throw rmm::logic_error if an error occurs during formatting */ template std::string formatted_log(std::string const& format, Args&&... args) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - auto size = static_cast(std::snprintf(nullptr, 0, format.c_str(), args...) + 1); - if (size <= 0) { throw std::runtime_error("Error during formatting."); } + auto size = std::snprintf(nullptr, 0, format.c_str(), args...) + 1; + RMM_EXPECTS(size >= 0, "Error during formatting."); + if (size == 0) { return {}; } + // NOLINTNEXTLINE(modernize-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays) std::unique_ptr buf(new char[size]); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) std::snprintf(buf.get(), size, format.c_str(), args...); From 99f41506e8514aed2d44b4a7c68ca3f014dbf7a6 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Tue, 12 Nov 2024 22:56:36 +0000 Subject: [PATCH 09/21] Remove unnecessary temporary creation in emplace_back --- tests/mr/device/arena_mr_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mr/device/arena_mr_tests.cpp b/tests/mr/device/arena_mr_tests.cpp index 95cc9c9c1..67f183a23 100644 --- a/tests/mr/device/arena_mr_tests.cpp +++ b/tests/mr/device/arena_mr_tests.cpp @@ -574,10 +574,10 @@ TEST_F(ArenaTest, DumpLogOnFailure) // NOLINT std::size_t num_threads{4}; threads.reserve(num_threads); for (std::size_t i = 0; i < num_threads; ++i) { - threads.emplace_back(std::thread([&] { + threads.emplace_back([&] { void* ptr = mr.allocate(32_KiB); mr.deallocate(ptr, 32_KiB); - })); + }); } for (auto& thread : threads) { From 7bde8d125e71ed05c071ef263f51e58ef6393cf5 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 00:48:37 +0000 Subject: [PATCH 10/21] Replace all uses of %d with more appropriate formats --- include/rmm/mr/device/detail/arena.hpp | 11 ++++++----- .../detail/stream_ordered_memory_resource.hpp | 14 +++++++------- include/rmm/mr/device/logging_resource_adaptor.hpp | 9 +++++---- include/rmm/mr/device/pool_memory_resource.hpp | 4 ++-- .../rmm/mr/device/tracking_resource_adaptor.hpp | 12 ++++++------ 5 files changed, 26 insertions(+), 24 deletions(-) diff --git a/include/rmm/mr/device/detail/arena.hpp b/include/rmm/mr/device/detail/arena.hpp index 23684a490..419c4fcf4 100644 --- a/include/rmm/mr/device/detail/arena.hpp +++ b/include/rmm/mr/device/detail/arena.hpp @@ -653,7 +653,7 @@ class global_arena final { logger->info(rmm::detail::formatted_log(" Arena size: %s", rmm::detail::format_bytes(upstream_block_.size()))); - logger->info(rmm::detail::formatted_log(" # superblocks: %d", superblocks_.size())); + logger->info(rmm::detail::formatted_log(" # superblocks: %zu", superblocks_.size())); if (!superblocks_.empty()) { logger->debug( rmm::detail::formatted_log(" Total size of superblocks: %s", @@ -665,20 +665,21 @@ class global_arena final { rmm::detail::format_bytes(total_free))); logger->info(rmm::detail::formatted_log(" Largest block of free memory: %s", rmm::detail::format_bytes(max_free))); - logger->info(rmm::detail::formatted_log(" Fragmentation: %0.2s", fragmentation)); + logger->info(rmm::detail::formatted_log(" Fragmentation: %0.2f", fragmentation)); - auto index = 0; + auto index = decltype(superblocks_.size()){0}; char* prev_end{}; for (auto const& sblk : superblocks_) { if (prev_end == nullptr) { prev_end = sblk.pointer(); } logger->debug(rmm::detail::formatted_log( - " Superblock %d: start=%p, end=%p, size=%d, empty=%d, # free blocks=%d, max free=%s, " + " Superblock %zu: start=%p, end=%p, size=%s, empty=%s, # free blocks=%zu, max " + "free=%s, " "gap=%s", index, sblk.pointer(), sblk.end(), rmm::detail::format_bytes(sblk.size()), - sblk.empty(), + sblk.empty() ? "T" : "F", sblk.free_blocks(), rmm::detail::format_bytes(sblk.max_free_size()), rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end)))); diff --git a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp index 519da2295..f177504f2 100644 --- a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp +++ b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp @@ -200,7 +200,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void* do_allocate(std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[A][stream %s][%dB]", rmm::detail::format_stream(stream), size); + RMM_LOG_TRACE("[A][stream %s][%zuB]", rmm::detail::format_stream(stream), size); if (size <= 0) { return nullptr; } @@ -214,7 +214,7 @@ class stream_ordered_memory_resource : public crtp, public device_ rmm::out_of_memory); auto const block = this->underlying().get_block(size, stream_event); - RMM_LOG_TRACE("[A][stream %s][%dB][%p]", + RMM_LOG_TRACE("[A][stream %s][%zuB][%p]", rmm::detail::format_stream(stream_event.stream), size, block.pointer()); @@ -233,7 +233,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void do_deallocate(void* ptr, std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[D][stream %s][%dB][%p]", rmm::detail::format_stream(stream), size, ptr); + RMM_LOG_TRACE("[D][stream %s][%zuB][%p]", rmm::detail::format_stream(stream), size, ptr); if (size <= 0 || ptr == nullptr) { return; } @@ -383,7 +383,7 @@ class stream_ordered_memory_resource : public crtp, public device_ if (merge_first) { merge_lists(stream_event, blocks, other_event, std::move(other_blocks)); - RMM_LOG_DEBUG("[A][Stream %s][%dB][Merged stream %s]", + RMM_LOG_DEBUG("[A][Stream %s][%zuB][Merged stream %s]", rmm::detail::format_stream(stream_event.stream), size, rmm::detail::format_stream(iter->first.stream)); @@ -413,8 +413,8 @@ class stream_ordered_memory_resource : public crtp, public device_ block_type const block = find_block(iter); if (block.is_valid()) { - RMM_LOG_DEBUG((merge_first) ? "[A][Stream %s][%dB][Found after merging stream %s]" - : "[A][Stream %s][%dB][Taken from stream %s]", + RMM_LOG_DEBUG((merge_first) ? "[A][Stream %s][%zuB][Found after merging stream %s]" + : "[A][Stream %s][%zuB][Taken from stream %s]", rmm::detail::format_stream(stream_event.stream), size, rmm::detail::format_stream(iter->first.stream)); @@ -470,7 +470,7 @@ class stream_ordered_memory_resource : public crtp, public device_ max_block = std::max(summary.first, max_block); free_mem += summary.second; }); - RMM_LOG_TRACE("[Summary][Free lists: %d][Blocks: %d][Max Block: %d][Total Free: %d]", + RMM_LOG_TRACE("[Summary][Free lists: %zu][Blocks: %zu][Max Block: %zu][Total Free: %zu]", stream_free_blocks_.size(), num_blocks, max_block, diff --git a/include/rmm/mr/device/logging_resource_adaptor.hpp b/include/rmm/mr/device/logging_resource_adaptor.hpp index 61b41d408..578543852 100644 --- a/include/rmm/mr/device/logging_resource_adaptor.hpp +++ b/include/rmm/mr/device/logging_resource_adaptor.hpp @@ -298,12 +298,12 @@ class logging_resource_adaptor final : public device_memory_resource { { try { auto const ptr = get_upstream_resource().allocate_async(bytes, stream); - logger_->info( - rmm::detail::formatted_log("allocate,%p,%lld,%lld", ptr, bytes, stream.value())); + logger_->info(rmm::detail::formatted_log( + "allocate,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream))); return ptr; } catch (...) { logger_->info(rmm::detail::formatted_log( - "allocate failure,%p,%lld,%lld", nullptr, bytes, stream.value())); + "allocate failure,%p,%zu,%s", nullptr, bytes, rmm::detail::format_stream(stream))); throw; } } @@ -324,7 +324,8 @@ class logging_resource_adaptor final : public device_memory_resource { */ void do_deallocate(void* ptr, std::size_t bytes, cuda_stream_view stream) override { - logger_->info(rmm::detail::formatted_log("free,%p,%d,%d", ptr, bytes, stream.value())); + logger_->info( + rmm::detail::formatted_log("free,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream))); get_upstream_resource().deallocate_async(ptr, bytes, stream); } diff --git a/include/rmm/mr/device/pool_memory_resource.hpp b/include/rmm/mr/device/pool_memory_resource.hpp index 4fff5a54c..037147de3 100644 --- a/include/rmm/mr/device/pool_memory_resource.hpp +++ b/include/rmm/mr/device/pool_memory_resource.hpp @@ -270,7 +270,7 @@ class pool_memory_resource final } try_size = std::max(min_size, try_size / 2); } - RMM_LOG_ERROR("[A][Stream %s][Upstream %dB][FAILURE maximum pool size exceeded]", + RMM_LOG_ERROR("[A][Stream %s][Upstream %zuB][FAILURE maximum pool size exceeded]", rmm::detail::format_stream(stream), min_size); RMM_FAIL("Maximum pool size exceeded", rmm::out_of_memory); @@ -350,7 +350,7 @@ class pool_memory_resource final */ std::optional block_from_upstream(std::size_t size, cuda_stream_view stream) { - RMM_LOG_DEBUG("[A][Stream %s][Upstream %dB]", rmm::detail::format_stream(stream), size); + RMM_LOG_DEBUG("[A][Stream %s][Upstream %zuB]", rmm::detail::format_stream(stream), size); if (size == 0) { return {}; } diff --git a/include/rmm/mr/device/tracking_resource_adaptor.hpp b/include/rmm/mr/device/tracking_resource_adaptor.hpp index 1c42e4222..8131eef4d 100644 --- a/include/rmm/mr/device/tracking_resource_adaptor.hpp +++ b/include/rmm/mr/device/tracking_resource_adaptor.hpp @@ -186,7 +186,7 @@ class tracking_resource_adaptor final : public device_memory_resource { void log_outstanding_allocations() const { #if SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG - RMM_LOG_DEBUG("Outstanding Allocations: %d", get_outstanding_allocations_str()); + RMM_LOG_DEBUG("Outstanding Allocations: %s", get_outstanding_allocations_str()); #endif // SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG } @@ -234,11 +234,11 @@ class tracking_resource_adaptor final : public device_memory_resource { // 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 + // Don't throw but log an error. Throwing in a destructor (or any noexcept) will call // std::terminate RMM_LOG_ERROR( - "Deallocating a pointer that was not tracked. Ptr: %p [$dB], Current Num. Allocations: " - "%d", + "Deallocating a pointer that was not tracked. Ptr: %p [%zuB], Current Num. Allocations: " + "%zu", ptr, bytes, this->allocations_.size()); @@ -248,10 +248,10 @@ class tracking_resource_adaptor final : public device_memory_resource { 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 + // Don't throw but log an error. Throwing in a destructor (or any noexcept) will call // std::terminate RMM_LOG_ERROR( - "Alloc bytes (%d) and Dealloc bytes (%d) do not match", allocated_bytes, bytes); + "Alloc bytes (%zu) and Dealloc bytes (%zu) do not match", allocated_bytes, bytes); bytes = allocated_bytes; } From 0df63a72360819824193cdf08b0ffc324e879cbe Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 00:52:09 +0000 Subject: [PATCH 11/21] Fix error in format_bytes, and make format_stream output hex --- include/rmm/detail/format.hpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index 47fe77df4..18d516c95 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -20,7 +20,9 @@ #include #include +#include #include +#include #include #include @@ -65,14 +67,15 @@ inline std::string format_bytes(std::size_t value) index++; } - return std::to_string(value) + ' ' + units.at(index); + return std::to_string(size) + ' ' + units.at(index); } // Stringify a stream ID inline std::string format_stream(rmm::cuda_stream_view stream) { - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - return std::to_string(reinterpret_cast(stream.value())); + std::stringstream sstr; + sstr << std::hex << stream.value(); + return sstr.str(); } } // namespace detail From e6f8be7621e36176aa4f98825091de86c0b0850e Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 00:54:04 +0000 Subject: [PATCH 12/21] use `c_str()` on all calls to `format_*` passed to %s --- include/rmm/detail/format.hpp | 3 ++- .../rmm/mr/device/arena_memory_resource.hpp | 2 +- include/rmm/mr/device/detail/arena.hpp | 20 +++++++++---------- .../detail/stream_ordered_memory_resource.hpp | 15 +++++++------- .../mr/device/logging_resource_adaptor.hpp | 8 ++++---- .../rmm/mr/device/pool_memory_resource.hpp | 5 +++-- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index 18d516c95..a51008a38 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -73,7 +74,7 @@ inline std::string format_bytes(std::size_t value) // Stringify a stream ID inline std::string format_stream(rmm::cuda_stream_view stream) { - std::stringstream sstr; + std::stringstream sstr{}; sstr << std::hex << stream.value(); return sstr.str(); } diff --git a/include/rmm/mr/device/arena_memory_resource.hpp b/include/rmm/mr/device/arena_memory_resource.hpp index d3a4bb09d..131f6542c 100644 --- a/include/rmm/mr/device/arena_memory_resource.hpp +++ b/include/rmm/mr/device/arena_memory_resource.hpp @@ -337,7 +337,7 @@ class arena_memory_resource final : public device_memory_resource { { logger_->info("**************************************************"); logger_->info(rmm::detail::formatted_log("Ran out of memory trying to allocate %s.", - rmm::detail::format_bytes(bytes))); + rmm::detail::format_bytes(bytes).c_str())); logger_->info("**************************************************"); logger_->info("Global arena:"); global_arena_.dump_memory_log(logger_); diff --git a/include/rmm/mr/device/detail/arena.hpp b/include/rmm/mr/device/detail/arena.hpp index 419c4fcf4..7e877b34d 100644 --- a/include/rmm/mr/device/detail/arena.hpp +++ b/include/rmm/mr/device/detail/arena.hpp @@ -651,20 +651,20 @@ class global_arena final { { std::lock_guard lock(mtx_); - logger->info(rmm::detail::formatted_log(" Arena size: %s", - rmm::detail::format_bytes(upstream_block_.size()))); + logger->info(rmm::detail::formatted_log( + " Arena size: %s", rmm::detail::format_bytes(upstream_block_.size()).c_str())); logger->info(rmm::detail::formatted_log(" # superblocks: %zu", superblocks_.size())); if (!superblocks_.empty()) { - logger->debug( - rmm::detail::formatted_log(" Total size of superblocks: %s", - rmm::detail::format_bytes(total_memory_size(superblocks_)))); + logger->debug(rmm::detail::formatted_log( + " Total size of superblocks: %s", + rmm::detail::format_bytes(total_memory_size(superblocks_)).c_str())); auto const total_free = total_free_size(superblocks_); auto const max_free = max_free_size(superblocks_); auto const fragmentation = (1 - max_free / static_cast(total_free)) * 100; logger->info(rmm::detail::formatted_log(" Total free memory: %s", - rmm::detail::format_bytes(total_free))); + rmm::detail::format_bytes(total_free).c_str())); logger->info(rmm::detail::formatted_log(" Largest block of free memory: %s", - rmm::detail::format_bytes(max_free))); + rmm::detail::format_bytes(max_free).c_str())); logger->info(rmm::detail::formatted_log(" Fragmentation: %0.2f", fragmentation)); auto index = decltype(superblocks_.size()){0}; @@ -678,11 +678,11 @@ class global_arena final { index, sblk.pointer(), sblk.end(), - rmm::detail::format_bytes(sblk.size()), + rmm::detail::format_bytes(sblk.size()).c_str(), sblk.empty() ? "T" : "F", sblk.free_blocks(), - rmm::detail::format_bytes(sblk.max_free_size()), - rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end)))); + rmm::detail::format_bytes(sblk.max_free_size()).c_str(), + rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end)).c_str())); prev_end = sblk.end(); index++; } diff --git a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp index f177504f2..cbfdb4e99 100644 --- a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp +++ b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp @@ -200,7 +200,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void* do_allocate(std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[A][stream %s][%zuB]", rmm::detail::format_stream(stream), size); + RMM_LOG_TRACE("[A][stream %s][%zuB]", rmm::detail::format_stream(stream).c_str(), size); if (size <= 0) { return nullptr; } @@ -215,7 +215,7 @@ class stream_ordered_memory_resource : public crtp, public device_ auto const block = this->underlying().get_block(size, stream_event); RMM_LOG_TRACE("[A][stream %s][%zuB][%p]", - rmm::detail::format_stream(stream_event.stream), + rmm::detail::format_stream(stream_event.stream).c_str(), size, block.pointer()); @@ -233,7 +233,8 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void do_deallocate(void* ptr, std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[D][stream %s][%zuB][%p]", rmm::detail::format_stream(stream), size, ptr); + RMM_LOG_TRACE( + "[D][stream %s][%zuB][%p]", rmm::detail::format_stream(stream).c_str(), size, ptr); if (size <= 0 || ptr == nullptr) { return; } @@ -384,9 +385,9 @@ class stream_ordered_memory_resource : public crtp, public device_ merge_lists(stream_event, blocks, other_event, std::move(other_blocks)); RMM_LOG_DEBUG("[A][Stream %s][%zuB][Merged stream %s]", - rmm::detail::format_stream(stream_event.stream), + rmm::detail::format_stream(stream_event.stream).c_str(), size, - rmm::detail::format_stream(iter->first.stream)); + rmm::detail::format_stream(iter->first.stream).c_str()); stream_free_blocks_.erase(iter); @@ -415,9 +416,9 @@ class stream_ordered_memory_resource : public crtp, public device_ if (block.is_valid()) { RMM_LOG_DEBUG((merge_first) ? "[A][Stream %s][%zuB][Found after merging stream %s]" : "[A][Stream %s][%zuB][Taken from stream %s]", - rmm::detail::format_stream(stream_event.stream), + rmm::detail::format_stream(stream_event.stream).c_str(), size, - rmm::detail::format_stream(iter->first.stream)); + rmm::detail::format_stream(iter->first.stream).c_str()); return block; } } diff --git a/include/rmm/mr/device/logging_resource_adaptor.hpp b/include/rmm/mr/device/logging_resource_adaptor.hpp index 578543852..3f667bc9e 100644 --- a/include/rmm/mr/device/logging_resource_adaptor.hpp +++ b/include/rmm/mr/device/logging_resource_adaptor.hpp @@ -299,11 +299,11 @@ class logging_resource_adaptor final : public device_memory_resource { try { auto const ptr = get_upstream_resource().allocate_async(bytes, stream); logger_->info(rmm::detail::formatted_log( - "allocate,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream))); + "allocate,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream).c_str())); return ptr; } catch (...) { logger_->info(rmm::detail::formatted_log( - "allocate failure,%p,%zu,%s", nullptr, bytes, rmm::detail::format_stream(stream))); + "allocate failure,%p,%zu,%s", nullptr, bytes, rmm::detail::format_stream(stream).c_str())); throw; } } @@ -324,8 +324,8 @@ class logging_resource_adaptor final : public device_memory_resource { */ void do_deallocate(void* ptr, std::size_t bytes, cuda_stream_view stream) override { - logger_->info( - rmm::detail::formatted_log("free,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream))); + logger_->info(rmm::detail::formatted_log( + "free,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream).c_str())); get_upstream_resource().deallocate_async(ptr, bytes, stream); } diff --git a/include/rmm/mr/device/pool_memory_resource.hpp b/include/rmm/mr/device/pool_memory_resource.hpp index 037147de3..b99d1f064 100644 --- a/include/rmm/mr/device/pool_memory_resource.hpp +++ b/include/rmm/mr/device/pool_memory_resource.hpp @@ -271,7 +271,7 @@ class pool_memory_resource final try_size = std::max(min_size, try_size / 2); } RMM_LOG_ERROR("[A][Stream %s][Upstream %zuB][FAILURE maximum pool size exceeded]", - rmm::detail::format_stream(stream), + rmm::detail::format_stream(stream).c_str(), min_size); RMM_FAIL("Maximum pool size exceeded", rmm::out_of_memory); } @@ -350,7 +350,8 @@ class pool_memory_resource final */ std::optional block_from_upstream(std::size_t size, cuda_stream_view stream) { - RMM_LOG_DEBUG("[A][Stream %s][Upstream %zuB]", rmm::detail::format_stream(stream), size); + RMM_LOG_DEBUG( + "[A][Stream %s][Upstream %zuB]", rmm::detail::format_stream(stream).c_str(), size); if (size == 0) { return {}; } From 60a8043a12a9ed60a7c94919646e4be31949e095 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 01:00:52 +0000 Subject: [PATCH 13/21] Remove deprecated use of logger() in replay bench --- benchmarks/replay/replay.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/benchmarks/replay/replay.cpp b/benchmarks/replay/replay.cpp index 5afed036a..7f45b7691 100644 --- a/benchmarks/replay/replay.cpp +++ b/benchmarks/replay/replay.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -172,7 +173,7 @@ struct replay_benchmark { void SetUp(const ::benchmark::State& state) { if (state.thread_index() == 0) { - rmm::logger().log(spdlog::level::info, "------ Start of Benchmark -----"); + RMM_LOG_INFO("------ Start of Benchmark -----"); mr_ = factory_(simulated_size_); } } @@ -181,7 +182,7 @@ struct replay_benchmark { void TearDown(const ::benchmark::State& state) { if (state.thread_index() == 0) { - rmm::logger().log(spdlog::level::info, "------ End of Benchmark -----"); + RMM_LOG_INFO("------ End of Benchmark -----"); // clean up any leaked allocations std::size_t total_leaked{0}; std::size_t num_leaked{0}; @@ -402,7 +403,7 @@ int main(int argc, char** argv) auto const num_threads = per_thread_events.size(); // Uncomment to enable / change default log level - // rmm::logger().set_level(spdlog::level::trace); + // rmm::detail::logger().set_level(spdlog::level::trace); if (args.count("resource") > 0) { std::string mr_name = args["resource"].as(); From 0de97c5bacb6e7bc939c4da0eacf6f6effe7d58f Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 01:02:38 +0000 Subject: [PATCH 14/21] Remove explicit fetching and linking of fmt in cmake --- CMakeLists.txt | 4 +--- cmake/thirdparty/get_fmt.cmake | 22 ---------------------- 2 files changed, 1 insertion(+), 25 deletions(-) delete mode 100644 cmake/thirdparty/get_fmt.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 26fcf1fd0..1f2a04f17 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,7 @@ rapids_cmake_build_type(Release) option(RMM_NVTX "Build RMM with NVTX support" OFF) option(BUILD_TESTS "Configure CMake to build tests" ON) option(BUILD_BENCHMARKS "Configure CMake to build (google) benchmarks" OFF) -# This is mostly so that dependent libraries, such as fmt, are configured in shared mode for +# This is mostly so that dependent libraries are configured in shared mode for # downstream dependents of RMM that get their common dependencies transitively. option(BUILD_SHARED_LIBS "Build RMM shared libraries" ON) set(RMM_LOGGING_LEVEL @@ -73,7 +73,6 @@ rapids_find_package( # add third party dependencies using CPM rapids_cpm_init() -include(cmake/thirdparty/get_fmt.cmake) include(cmake/thirdparty/get_spdlog.cmake) include(cmake/thirdparty/get_cccl.cmake) include(cmake/thirdparty/get_nvtx.cmake) @@ -96,7 +95,6 @@ else() endif() target_link_libraries(rmm INTERFACE CCCL::CCCL) -target_link_libraries(rmm INTERFACE fmt::fmt-header-only) target_link_libraries(rmm INTERFACE spdlog::spdlog_header_only) target_link_libraries(rmm INTERFACE dl) target_link_libraries(rmm INTERFACE nvtx3::nvtx3-cpp) diff --git a/cmake/thirdparty/get_fmt.cmake b/cmake/thirdparty/get_fmt.cmake deleted file mode 100644 index 5787fb73f..000000000 --- a/cmake/thirdparty/get_fmt.cmake +++ /dev/null @@ -1,22 +0,0 @@ -# ============================================================================= -# Copyright (c) 2023, 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. -# ============================================================================= - -# Use CPM to find or clone fmt -function(find_and_configure_fmt) - - include(${rapids-cmake-dir}/cpm/fmt.cmake) - rapids_cpm_fmt(INSTALL_EXPORT_SET rmm-exports BUILD_EXPORT_SET rmm-exports) -endfunction() - -find_and_configure_fmt() From 4f18c40b3453bf22376c187dc302804647ab8519 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 01:11:53 +0000 Subject: [PATCH 15/21] Style --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1f2a04f17..44d7fbb79 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,8 +41,8 @@ rapids_cmake_build_type(Release) option(RMM_NVTX "Build RMM with NVTX support" OFF) option(BUILD_TESTS "Configure CMake to build tests" ON) option(BUILD_BENCHMARKS "Configure CMake to build (google) benchmarks" OFF) -# This is mostly so that dependent libraries are configured in shared mode for -# downstream dependents of RMM that get their common dependencies transitively. +# This is mostly so that dependent libraries are configured in shared mode for downstream dependents +# of RMM that get their common dependencies transitively. option(BUILD_SHARED_LIBS "Build RMM shared libraries" ON) set(RMM_LOGGING_LEVEL "INFO" From 89fceea2529b117a60f443fa9ad69a88103da8bc Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 02:42:19 +0000 Subject: [PATCH 16/21] Specialize formatted_log for string without arguments (fix potential security error). --- include/rmm/detail/format.hpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index a51008a38..509d066fe 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -56,6 +56,13 @@ std::string formatted_log(std::string const& format, Args&&... args) return {buf.get(), buf.get() + size - 1}; // drop '\0' } +// specialization for no arguments +template <> +inline std::string formatted_log(std::string const& format) +{ + return format; +} + // Stringify a size in bytes to a human-readable value inline std::string format_bytes(std::size_t value) { From 5330063f6463fe05263e114a18fb5d1b9c3c4147 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 04:23:12 +0000 Subject: [PATCH 17/21] Update intersphinx_mapping for new cuda.py docs --- python/rmm/docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/rmm/docs/conf.py b/python/rmm/docs/conf.py index 0b2c21d5a..5905aa4ab 100644 --- a/python/rmm/docs/conf.py +++ b/python/rmm/docs/conf.py @@ -197,7 +197,7 @@ intersphinx_mapping = { "python": ("https://docs.python.org/3", None), "numba": ("https://numba.readthedocs.io/en/stable", None), - "cuda-python": ("https://nvidia.github.io/cuda-python/", None), + "cuda-python": ("https://nvidia.github.io/cuda-python/cuda-bindings/", None), } # Config numpydoc From 00d66a50c5de6609e30d1aed1764a11f08a513d5 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 05:00:29 +0000 Subject: [PATCH 18/21] style --- python/rmm/docs/conf.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/rmm/docs/conf.py b/python/rmm/docs/conf.py index 5905aa4ab..2aad3a82c 100644 --- a/python/rmm/docs/conf.py +++ b/python/rmm/docs/conf.py @@ -197,7 +197,10 @@ intersphinx_mapping = { "python": ("https://docs.python.org/3", None), "numba": ("https://numba.readthedocs.io/en/stable", None), - "cuda-python": ("https://nvidia.github.io/cuda-python/cuda-bindings/", None), + "cuda-python": ( + "https://nvidia.github.io/cuda-python/cuda-bindings/", + None, + ), } # Config numpydoc From 34fa43059e51ed295a676a6cfe6167d091d86b71 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Wed, 13 Nov 2024 10:56:36 +0000 Subject: [PATCH 19/21] Fix potential for warnings --- include/rmm/detail/format.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index 509d066fe..a6fc0a790 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -46,9 +46,10 @@ template std::string formatted_log(std::string const& format, Args&&... args) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - auto size = std::snprintf(nullptr, 0, format.c_str(), args...) + 1; - RMM_EXPECTS(size >= 0, "Error during formatting."); - if (size == 0) { return {}; } + auto retsize = std::snprintf(nullptr, 0, format.c_str(), args...); + RMM_EXPECTS(retsize >= 0, "Error during formatting."); + if (retsize == 0) { return {}; } + auto size = static_cast(retsize) + 1; // for null terminator // NOLINTNEXTLINE(modernize-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays) std::unique_ptr buf(new char[size]); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) From fa9dbc3701f9453d4ed528cb8f1c4f5708d126f7 Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 14 Nov 2024 00:47:34 +0000 Subject: [PATCH 20/21] Automatically support std::string arguments to formatted_log --- include/rmm/detail/format.hpp | 18 +++++++++++++---- .../rmm/mr/device/arena_memory_resource.hpp | 2 +- include/rmm/mr/device/detail/arena.hpp | 20 +++++++++---------- .../detail/stream_ordered_memory_resource.hpp | 15 +++++++------- .../mr/device/logging_resource_adaptor.hpp | 8 ++++---- .../rmm/mr/device/pool_memory_resource.hpp | 5 ++--- 6 files changed, 38 insertions(+), 30 deletions(-) diff --git a/include/rmm/detail/format.hpp b/include/rmm/detail/format.hpp index a6fc0a790..21acac032 100644 --- a/include/rmm/detail/format.hpp +++ b/include/rmm/detail/format.hpp @@ -45,15 +45,25 @@ namespace detail { template std::string formatted_log(std::string const& format, Args&&... args) { - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - auto retsize = std::snprintf(nullptr, 0, format.c_str(), args...); + auto convert_to_c_string = [](auto&& arg) -> decltype(auto) { + using ArgType = std::decay_t; + if constexpr (std::is_same_v) { + return arg.c_str(); + } else { + return std::forward(arg); + } + }; + + // NOLINTBEGIN(cppcoreguidelines-pro-type-vararg) + auto retsize = + std::snprintf(nullptr, 0, format.c_str(), convert_to_c_string(std::forward(args))...); RMM_EXPECTS(retsize >= 0, "Error during formatting."); if (retsize == 0) { return {}; } auto size = static_cast(retsize) + 1; // for null terminator // NOLINTNEXTLINE(modernize-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays) std::unique_ptr buf(new char[size]); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - std::snprintf(buf.get(), size, format.c_str(), args...); + std::snprintf(buf.get(), size, format.c_str(), convert_to_c_string(std::forward(args))...); + // NOLINTEND(cppcoreguidelines-pro-type-vararg) return {buf.get(), buf.get() + size - 1}; // drop '\0' } diff --git a/include/rmm/mr/device/arena_memory_resource.hpp b/include/rmm/mr/device/arena_memory_resource.hpp index 131f6542c..d3a4bb09d 100644 --- a/include/rmm/mr/device/arena_memory_resource.hpp +++ b/include/rmm/mr/device/arena_memory_resource.hpp @@ -337,7 +337,7 @@ class arena_memory_resource final : public device_memory_resource { { logger_->info("**************************************************"); logger_->info(rmm::detail::formatted_log("Ran out of memory trying to allocate %s.", - rmm::detail::format_bytes(bytes).c_str())); + rmm::detail::format_bytes(bytes))); logger_->info("**************************************************"); logger_->info("Global arena:"); global_arena_.dump_memory_log(logger_); diff --git a/include/rmm/mr/device/detail/arena.hpp b/include/rmm/mr/device/detail/arena.hpp index 7e877b34d..419c4fcf4 100644 --- a/include/rmm/mr/device/detail/arena.hpp +++ b/include/rmm/mr/device/detail/arena.hpp @@ -651,20 +651,20 @@ class global_arena final { { std::lock_guard lock(mtx_); - logger->info(rmm::detail::formatted_log( - " Arena size: %s", rmm::detail::format_bytes(upstream_block_.size()).c_str())); + logger->info(rmm::detail::formatted_log(" Arena size: %s", + rmm::detail::format_bytes(upstream_block_.size()))); logger->info(rmm::detail::formatted_log(" # superblocks: %zu", superblocks_.size())); if (!superblocks_.empty()) { - logger->debug(rmm::detail::formatted_log( - " Total size of superblocks: %s", - rmm::detail::format_bytes(total_memory_size(superblocks_)).c_str())); + logger->debug( + rmm::detail::formatted_log(" Total size of superblocks: %s", + rmm::detail::format_bytes(total_memory_size(superblocks_)))); auto const total_free = total_free_size(superblocks_); auto const max_free = max_free_size(superblocks_); auto const fragmentation = (1 - max_free / static_cast(total_free)) * 100; logger->info(rmm::detail::formatted_log(" Total free memory: %s", - rmm::detail::format_bytes(total_free).c_str())); + rmm::detail::format_bytes(total_free))); logger->info(rmm::detail::formatted_log(" Largest block of free memory: %s", - rmm::detail::format_bytes(max_free).c_str())); + rmm::detail::format_bytes(max_free))); logger->info(rmm::detail::formatted_log(" Fragmentation: %0.2f", fragmentation)); auto index = decltype(superblocks_.size()){0}; @@ -678,11 +678,11 @@ class global_arena final { index, sblk.pointer(), sblk.end(), - rmm::detail::format_bytes(sblk.size()).c_str(), + rmm::detail::format_bytes(sblk.size()), sblk.empty() ? "T" : "F", sblk.free_blocks(), - rmm::detail::format_bytes(sblk.max_free_size()).c_str(), - rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end)).c_str())); + rmm::detail::format_bytes(sblk.max_free_size()), + rmm::detail::format_bytes(static_cast(sblk.pointer() - prev_end)))); prev_end = sblk.end(); index++; } diff --git a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp index cbfdb4e99..f177504f2 100644 --- a/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp +++ b/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp @@ -200,7 +200,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void* do_allocate(std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE("[A][stream %s][%zuB]", rmm::detail::format_stream(stream).c_str(), size); + RMM_LOG_TRACE("[A][stream %s][%zuB]", rmm::detail::format_stream(stream), size); if (size <= 0) { return nullptr; } @@ -215,7 +215,7 @@ class stream_ordered_memory_resource : public crtp, public device_ auto const block = this->underlying().get_block(size, stream_event); RMM_LOG_TRACE("[A][stream %s][%zuB][%p]", - rmm::detail::format_stream(stream_event.stream).c_str(), + rmm::detail::format_stream(stream_event.stream), size, block.pointer()); @@ -233,8 +233,7 @@ class stream_ordered_memory_resource : public crtp, public device_ */ void do_deallocate(void* ptr, std::size_t size, cuda_stream_view stream) override { - RMM_LOG_TRACE( - "[D][stream %s][%zuB][%p]", rmm::detail::format_stream(stream).c_str(), size, ptr); + RMM_LOG_TRACE("[D][stream %s][%zuB][%p]", rmm::detail::format_stream(stream), size, ptr); if (size <= 0 || ptr == nullptr) { return; } @@ -385,9 +384,9 @@ class stream_ordered_memory_resource : public crtp, public device_ merge_lists(stream_event, blocks, other_event, std::move(other_blocks)); RMM_LOG_DEBUG("[A][Stream %s][%zuB][Merged stream %s]", - rmm::detail::format_stream(stream_event.stream).c_str(), + rmm::detail::format_stream(stream_event.stream), size, - rmm::detail::format_stream(iter->first.stream).c_str()); + rmm::detail::format_stream(iter->first.stream)); stream_free_blocks_.erase(iter); @@ -416,9 +415,9 @@ class stream_ordered_memory_resource : public crtp, public device_ if (block.is_valid()) { RMM_LOG_DEBUG((merge_first) ? "[A][Stream %s][%zuB][Found after merging stream %s]" : "[A][Stream %s][%zuB][Taken from stream %s]", - rmm::detail::format_stream(stream_event.stream).c_str(), + rmm::detail::format_stream(stream_event.stream), size, - rmm::detail::format_stream(iter->first.stream).c_str()); + rmm::detail::format_stream(iter->first.stream)); return block; } } diff --git a/include/rmm/mr/device/logging_resource_adaptor.hpp b/include/rmm/mr/device/logging_resource_adaptor.hpp index 3f667bc9e..578543852 100644 --- a/include/rmm/mr/device/logging_resource_adaptor.hpp +++ b/include/rmm/mr/device/logging_resource_adaptor.hpp @@ -299,11 +299,11 @@ class logging_resource_adaptor final : public device_memory_resource { try { auto const ptr = get_upstream_resource().allocate_async(bytes, stream); logger_->info(rmm::detail::formatted_log( - "allocate,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream).c_str())); + "allocate,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream))); return ptr; } catch (...) { logger_->info(rmm::detail::formatted_log( - "allocate failure,%p,%zu,%s", nullptr, bytes, rmm::detail::format_stream(stream).c_str())); + "allocate failure,%p,%zu,%s", nullptr, bytes, rmm::detail::format_stream(stream))); throw; } } @@ -324,8 +324,8 @@ class logging_resource_adaptor final : public device_memory_resource { */ void do_deallocate(void* ptr, std::size_t bytes, cuda_stream_view stream) override { - logger_->info(rmm::detail::formatted_log( - "free,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream).c_str())); + logger_->info( + rmm::detail::formatted_log("free,%p,%zu,%s", ptr, bytes, rmm::detail::format_stream(stream))); get_upstream_resource().deallocate_async(ptr, bytes, stream); } diff --git a/include/rmm/mr/device/pool_memory_resource.hpp b/include/rmm/mr/device/pool_memory_resource.hpp index b99d1f064..037147de3 100644 --- a/include/rmm/mr/device/pool_memory_resource.hpp +++ b/include/rmm/mr/device/pool_memory_resource.hpp @@ -271,7 +271,7 @@ class pool_memory_resource final try_size = std::max(min_size, try_size / 2); } RMM_LOG_ERROR("[A][Stream %s][Upstream %zuB][FAILURE maximum pool size exceeded]", - rmm::detail::format_stream(stream).c_str(), + rmm::detail::format_stream(stream), min_size); RMM_FAIL("Maximum pool size exceeded", rmm::out_of_memory); } @@ -350,8 +350,7 @@ class pool_memory_resource final */ std::optional block_from_upstream(std::size_t size, cuda_stream_view stream) { - RMM_LOG_DEBUG( - "[A][Stream %s][Upstream %zuB]", rmm::detail::format_stream(stream).c_str(), size); + RMM_LOG_DEBUG("[A][Stream %s][Upstream %zuB]", rmm::detail::format_stream(stream), size); if (size == 0) { return {}; } From 702cbc60c34f96ec12a4507617f765cf58dc536e Mon Sep 17 00:00:00 2001 From: Mark Harris Date: Thu, 14 Nov 2024 00:51:58 +0000 Subject: [PATCH 21/21] Remove unnecessary added include --- tests/logger_tests.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/logger_tests.cpp b/tests/logger_tests.cpp index 7148c2d55..8a5d37be2 100644 --- a/tests/logger_tests.cpp +++ b/tests/logger_tests.cpp @@ -23,7 +23,6 @@ #include #include -#include #include #include #include