Skip to content

Commit

Permalink
Remove all explicit usage of fmtlib (#1724)
Browse files Browse the repository at this point in the history
Fixes #1717
Also fixes #1710 in 5330063

I have replaced fmt-style format string placeholders (`"... {} ..."`) with printf-style placeholders by adding a function `rmm::detail::formatted_log()`, which I modified from @vyasr 's #1722.

~The only remaining mention of fmt is in CMakeLists.txt. Do we still need to explicitly fetch fmt?~
Removed.

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

Approvers:
  - Lawrence Mitchell (https://github.com/wence-)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1724
  • Loading branch information
harrism authored Nov 14, 2024
1 parent 220003e commit 52d61c5
Show file tree
Hide file tree
Showing 17 changed files with 189 additions and 136 deletions.
6 changes: 2 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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, such as fmt, 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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions benchmarks/replay/replay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <rmm/cuda_stream_view.hpp>
#include <rmm/detail/error.hpp>
#include <rmm/logger.hpp>
#include <rmm/mr/device/arena_memory_resource.hpp>
#include <rmm/mr/device/binning_memory_resource.hpp>
#include <rmm/mr/device/cuda_memory_resource.hpp>
Expand Down
4 changes: 2 additions & 2 deletions benchmarks/utilities/log_parser.hpp
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -151,7 +151,7 @@ inline std::vector<event> 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<uintptr_t> pointers = csv.GetColumn<uintptr_t>("Pointer", parse_pointer);
Expand Down
22 changes: 0 additions & 22 deletions cmake/thirdparty/get_fmt.cmake

This file was deleted.

101 changes: 101 additions & 0 deletions include/rmm/detail/format.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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 <rmm/cuda_stream_view.hpp>

#include <array>
#include <cstdio>
#include <ios>
#include <iostream>
#include <memory>
#include <sstream>
#include <stdexcept>
#include <string>

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
* @return The formatted message
* @throw rmm::logic_error if an error occurs during formatting
*/
template <typename... Args>
std::string formatted_log(std::string const& format, Args&&... args)
{
auto convert_to_c_string = [](auto&& arg) -> decltype(auto) {
using ArgType = std::decay_t<decltype(arg)>;
if constexpr (std::is_same_v<ArgType, std::string>) {
return arg.c_str();
} else {
return std::forward<decltype(arg)>(arg);
}
};

// NOLINTBEGIN(cppcoreguidelines-pro-type-vararg)
auto retsize =
std::snprintf(nullptr, 0, format.c_str(), convert_to_c_string(std::forward<Args>(args))...);
RMM_EXPECTS(retsize >= 0, "Error during formatting.");
if (retsize == 0) { return {}; }
auto size = static_cast<std::size_t>(retsize) + 1; // for null terminator
// NOLINTNEXTLINE(modernize-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays)
std::unique_ptr<char[]> buf(new char[size]);
std::snprintf(buf.get(), size, format.c_str(), convert_to_c_string(std::forward<Args>(args))...);
// NOLINTEND(cppcoreguidelines-pro-type-vararg)
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)
{
static std::array units{"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"};

int index = 0;
auto size = static_cast<double>(value);
while (size > 1024) {
size /= 1024;
index++;
}

return std::to_string(size) + ' ' + units.at(index);
}

// Stringify a stream ID
inline std::string format_stream(rmm::cuda_stream_view stream)
{
std::stringstream sstr{};
sstr << std::hex << stream.value();
return sstr.str();
}

} // namespace detail
} // namespace RMM_NAMESPACE
4 changes: 2 additions & 2 deletions include/rmm/detail/logging_assert.hpp
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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); \
} \
Expand Down
55 changes: 14 additions & 41 deletions include/rmm/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@

#pragma once

#include <rmm/cuda_stream_view.hpp>
#include <rmm/detail/export.hpp>
#include <rmm/detail/format.hpp>

#include <fmt/format.h>
#include <fmt/ostream.h>
#include <spdlog/sinks/basic_file_sink.h>
#include <spdlog/spdlog.h>

#include <array>
#include <iostream>
#include <string>

namespace RMM_NAMESPACE {
Expand Down Expand Up @@ -70,32 +68,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<double>(value.value);
while (size > 1024) {
size /= 1024;
index++;
}
return os << size << ' ' << units.at(index);
}
};

inline spdlog::logger& logger()
{
static detail::logger_wrapper wrapped{};
Expand Down Expand Up @@ -125,20 +97,21 @@ 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

} // namespace RMM_NAMESPACE

// Doxygen doesn't like this because we're overloading something from fmt
//! @cond Doxygen_Suppress
template <>
struct fmt::formatter<rmm::detail::bytes> : fmt::ostream_formatter {};

//! @endcond
4 changes: 3 additions & 1 deletion include/rmm/mr/device/arena_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <rmm/aligned.hpp>
#include <rmm/detail/error.hpp>
#include <rmm/detail/export.hpp>
#include <rmm/detail/format.hpp>
#include <rmm/detail/logging_assert.hpp>
#include <rmm/logger.hpp>
#include <rmm/mr/device/detail/arena.hpp>
Expand Down Expand Up @@ -335,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::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_);
Expand Down
41 changes: 23 additions & 18 deletions include/rmm/mr/device/detail/arena.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
#include <rmm/cuda_stream_view.hpp>
#include <rmm/detail/error.hpp>
#include <rmm/detail/export.hpp>
#include <rmm/detail/format.hpp>
#include <rmm/detail/logging_assert.hpp>
#include <rmm/logger.hpp>
#include <rmm/resource_ref.hpp>

#include <cuda_runtime_api.h>

#include <fmt/core.h>
#include <spdlog/common.h>

#include <algorithm>
Expand Down Expand Up @@ -651,33 +651,38 @@ class global_arena final {
{
std::lock_guard lock(mtx_);

logger->info(" Arena size: {}", rmm::detail::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: %zu", superblocks_.size()));
if (!superblocks_.empty()) {
logger->debug(" Total size of superblocks: {}",
rmm::detail::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<double>(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(" 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.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(
" Superblock {}: start={}, end={}, size={}, empty={}, # free blocks={}, max free={}, "
"gap={}",
logger->debug(rmm::detail::formatted_log(
" Superblock %zu: start=%p, end=%p, size=%s, empty=%s, # free blocks=%zu, max "
"free=%s, "
"gap=%s",
index,
fmt::ptr(sblk.pointer()),
fmt::ptr(sblk.end()),
rmm::detail::bytes{sblk.size()},
sblk.empty(),
sblk.pointer(),
sblk.end(),
rmm::detail::format_bytes(sblk.size()),
sblk.empty() ? "T" : "F",
sblk.free_blocks(),
rmm::detail::bytes{sblk.max_free_size()},
rmm::detail::bytes{static_cast<size_t>(sblk.pointer() - prev_end)});
rmm::detail::format_bytes(sblk.max_free_size()),
rmm::detail::format_bytes(static_cast<size_t>(sblk.pointer() - prev_end))));
prev_end = sblk.end();
index++;
}
Expand Down
9 changes: 2 additions & 7 deletions include/rmm/mr/device/detail/coalescing_free_list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
#include <rmm/detail/export.hpp>
#include <rmm/mr/device/detail/free_list.hpp>

#include <fmt/core.h>

#include <algorithm>
#include <cassert>
#include <cstddef>
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
Loading

0 comments on commit 52d61c5

Please sign in to comment.