Skip to content

Commit

Permalink
Create logger wrapper around spdlog that can be easily reused in othe…
Browse files Browse the repository at this point in the history
…r libraries (#1722)

This PR defines a new way to produce a logger wrapping spdlog. The logger's interface is declared in a template header file that can be processed by CMake to produce an interface that may be customized for placement into any project. The new implementation uses the PImpl idiom to isolate the spdlog (and transitively, fmt) dependency from the public API of the logger. The implementation is defined in an impl header. A corresponding source template file is provided that simply includes this header. All of these files are wrapped in some CMake logic for producing a custom target for a given project.

rmm leverages this new logger by requesting the creation of a logger target and a corresponding implementation. This is a breaking change because consumers of rmm will need to link the new `rmm_logger_impl` target into their own libraries to get logging support. Once this gets merged, the plan is to move this implementation out of rmm into its own repository. At that point, the logger may also be used to completely replace logger implementations in cudf, raft, and cuml (as well as any other RAPIDS libraries that are aiming to provide their own logging implementation). Once everything in RAPIDS is migrated to using the new logger, we will update the way that it uses spdlog to completely hide all spdlog symbols, which solves a half dozen different problems for us when it comes to packaging (symbol collision issues, ABI compatibility, conda environment conflicts, bundling of headers into conda packages, etc).

Resolves #1709

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Mark Harris (https://github.com/harrism)
  - Jake Awe (https://github.com/AyodeAwe)

URL: #1722
  • Loading branch information
vyasr authored Nov 26, 2024
1 parent d54a28a commit 4cfa31f
Show file tree
Hide file tree
Showing 32 changed files with 1,068 additions and 364 deletions.
6 changes: 4 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ rapids_find_package(
# add third party dependencies using CPM
rapids_cpm_init()

include(cmake/thirdparty/get_spdlog.cmake)
add_subdirectory(rapids_logger)
rapids_make_logger(rmm EXPORT_SET rmm-exports)

include(cmake/thirdparty/get_cccl.cmake)
include(cmake/thirdparty/get_nvtx.cmake)

Expand All @@ -93,8 +95,8 @@ else()
target_link_libraries(rmm INTERFACE CUDA::cudart)
endif()

target_link_libraries(rmm INTERFACE rmm_logger)
target_link_libraries(rmm INTERFACE CCCL::CCCL)
target_link_libraries(rmm INTERFACE spdlog::spdlog_header_only)
target_link_libraries(rmm INTERFACE dl)
target_link_libraries(rmm INTERFACE nvtx3::nvtx3-cpp)
target_compile_features(rmm INTERFACE cxx_std_17 $<BUILD_INTERFACE:cuda_std_17>)
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,8 @@ of more detailed logging. The default is `INFO`. Available levels are `TRACE`, `
The log relies on the [spdlog](https://github.com/gabime/spdlog.git) library.
Note that to see logging below the `INFO` level, the application must also set the logging level at
run time. C++ applications must must call `rmm::logger().set_level()`, for example to enable all
levels of logging down to `TRACE`, call `rmm::logger().set_level(spdlog::level::trace)` (and compile
run time. C++ applications must must call `rmm::default_logger().set_level()`, for example to enable all
levels of logging down to `TRACE`, call `rmm::default_logger().set_level(spdlog::level::trace)` (and compile
librmm with `-DRMM_LOGGING_LEVEL=TRACE`). Python applications must call `rmm.set_logging_level()`,
for example to enable all levels of logging down to `TRACE`, call `rmm.set_logging_level("trace")`
(and compile the RMM Python module with `-DRMM_LOGGING_LEVEL=TRACE`).
Expand Down
11 changes: 9 additions & 2 deletions benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,19 @@ function(ConfigureBench BENCH_NAME)
RUNTIME_OUTPUT_DIRECTORY "$<BUILD_INTERFACE:${RMM_BINARY_DIR}/gbenchmarks>"
CUDA_ARCHITECTURES "${CMAKE_CUDA_ARCHITECTURES}"
INSTALL_RPATH "\$ORIGIN/../../../lib")
target_link_libraries(${BENCH_NAME} benchmark::benchmark pthread rmm)
target_link_libraries(${BENCH_NAME} PRIVATE benchmark::benchmark pthread rmm)
target_compile_definitions(${BENCH_NAME}
PUBLIC "SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${RMM_LOGGING_LEVEL}")
PUBLIC "RMM_LOG_ACTIVE_LEVEL=RMM_LOG_LEVEL_${RMM_LOGGING_LEVEL}")

if(PER_THREAD_DEFAULT_STREAM)
target_compile_definitions(${BENCH_NAME} PUBLIC CUDA_API_PER_THREAD_DEFAULT_STREAM)
endif()

target_compile_options(${BENCH_NAME} PUBLIC $<$<COMPILE_LANG_AND_ID:CXX,GNU,Clang>:-Wall -Werror
-Wno-unknown-pragmas>)

target_link_libraries(${BENCH_NAME} PRIVATE rmm_bench_logger)

if(DISABLE_DEPRECATION_WARNING)
target_compile_options(
${BENCH_NAME} PUBLIC $<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=-Wno-deprecated-declarations>)
Expand All @@ -58,6 +61,10 @@ function(ConfigureBench BENCH_NAME)
EXCLUDE_FROM_ALL)
endfunction(ConfigureBench)

# Create an object library for the logger so that we don't have to recompile it.
add_library(rmm_bench_logger OBJECT)
target_link_libraries(rmm_bench_logger PRIVATE rmm_logger_impl)

# random allocations benchmark
ConfigureBench(RANDOM_ALLOCATIONS_BENCH random_allocations/random_allocations.cpp)

Expand Down
4 changes: 2 additions & 2 deletions benchmarks/replay/replay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@
#include <benchmarks/utilities/cxxopts.hpp>
#include <benchmarks/utilities/log_parser.hpp>
#include <benchmarks/utilities/simulated_memory_resource.hpp>
#include <spdlog/common.h>

#include <atomic>
#include <chrono>
#include <condition_variable>
#include <iterator>
#include <memory>
#include <numeric>
Expand Down Expand Up @@ -403,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::detail::logger().set_level(spdlog::level::trace);
// rmm::logger().set_level(rmm::level_enum::trace);

if (args.count("resource") > 0) {
std::string mr_name = args["resource"].as<std::string>();
Expand Down
23 changes: 2 additions & 21 deletions ci/check_symbols.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,13 @@ for dso_file in ${dso_files}; do
echo " * WEAK: $(grep --count -E ' WEAK ' < ${symbol_file})"
echo " * LOCAL: $(grep --count -E ' LOCAL ' < ${symbol_file})"

# Explanation for '-v' uses here:
#
# * 'format_error' symbols are intentionally exported, that type of error
# can be thrown across library boundaries. See "Problems with C++ exceptions"
# at https://gcc.gnu.org/wiki/Visibility.
echo "checking for 'fmt::' symbols..."
if grep -E 'fmt\:\:' < "${symbol_file}" \
| grep -v 'format_error'
then
if grep -E 'fmt\:\:' < "${symbol_file}"; then
raise-symbols-found-error 'fmt::'
fi

# Explanation for '-v' uses here:
#
# * trivially-destructible objects sometimes get an entry in the symbol table
# for a specialization of `std::_Destroy_aux()` called to destroy them.
# There is one for `spdlog::details::log_msg_buffer like that:
#
# 'std::_Destroy_aux<false>::__destroy<spdlog::details::log_msg_buffer*>'
#
# That should be safe to export.
#
echo "checking for 'spdlog::' symbols..."
if grep -E 'spdlog\:\:' < "${symbol_file}" \
| grep -v 'std\:\:_Destroy_aux'
then
if grep -E 'spdlog\:\:' < "${symbol_file}"; then
raise-symbols-found-error 'spdlog::'
fi
echo "No symbol visibility issues found"
Expand Down
3 changes: 1 addition & 2 deletions cmake/thirdparty/get_spdlog.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@
# the License.
# =============================================================================

# Use CPM to find or clone speedlog
# Use CPM to find or clone speedlog.
function(find_and_configure_spdlog)

include(${rapids-cmake-dir}/cpm/spdlog.cmake)
rapids_cpm_spdlog(
FMT_OPTION "EXTERNAL_FMT_HO"
INSTALL_EXPORT_SET rmm-exports
BUILD_EXPORT_SET rmm-exports)

endfunction()

find_and_configure_spdlog()
48 changes: 0 additions & 48 deletions include/rmm/detail/format.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,60 +20,12 @@

#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)
{
Expand Down
2 changes: 1 addition & 1 deletion include/rmm/detail/logging_assert.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*/
#ifdef NDEBUG
#define RMM_LOGGING_ASSERT(_expr) (void)0
#elif SPDLOG_ACTIVE_LEVEL < SPDLOG_LEVEL_OFF
#elif RMM_LOG_ACTIVE_LEVEL < RMM_LOG_LEVEL_OFF
#define RMM_LOGGING_ASSERT(_expr) \
do { \
bool const success = (_expr); \
Expand Down
117 changes: 0 additions & 117 deletions include/rmm/logger.hpp

This file was deleted.

26 changes: 6 additions & 20 deletions include/rmm/mr/device/arena_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@

#include <cuda_runtime_api.h>

#include <spdlog/common.h>

#include <cstddef>
#include <map>
#include <shared_mutex>
Expand Down Expand Up @@ -98,12 +96,9 @@ class arena_memory_resource final : public device_memory_resource {
: global_arena_{upstream_mr, arena_size}, dump_log_on_failure_{dump_log_on_failure}
{
if (dump_log_on_failure_) {
logger_ =
std::make_shared<spdlog::logger>("arena_memory_dump",
std::make_shared<spdlog::sinks::basic_file_sink_mt>(
"rmm_arena_memory_dump.log", true /*truncate file*/));
logger_ = std::make_shared<logger>("arena_memory_dump", "rmm_arena_memory_dump.log");
// Set the level to `debug` for more detailed output.
logger_->set_level(spdlog::level::info);
logger_->set_level(level_enum::info);
}
}

Expand All @@ -120,17 +115,9 @@ class arena_memory_resource final : public device_memory_resource {
explicit arena_memory_resource(Upstream* upstream_mr,
std::optional<std::size_t> arena_size = std::nullopt,
bool dump_log_on_failure = false)
: global_arena_{to_device_async_resource_ref_checked(upstream_mr), arena_size},
dump_log_on_failure_{dump_log_on_failure}
: arena_memory_resource{
to_device_async_resource_ref_checked(upstream_mr), arena_size, dump_log_on_failure}
{
if (dump_log_on_failure_) {
logger_ =
std::make_shared<spdlog::logger>("arena_memory_dump",
std::make_shared<spdlog::sinks::basic_file_sink_mt>(
"rmm_arena_memory_dump.log", true /*truncate file*/));
// Set the level to `debug` for more detailed output.
logger_->set_level(spdlog::level::info);
}
}

~arena_memory_resource() override = default;
Expand Down Expand Up @@ -336,8 +323,7 @@ class arena_memory_resource final : public device_memory_resource {
void dump_memory_log(size_t bytes)
{
logger_->info("**************************************************");
logger_->info(rmm::detail::formatted_log("Ran out of memory trying to allocate %s.",
rmm::detail::format_bytes(bytes)));
logger_->info("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 Expand Up @@ -366,7 +352,7 @@ class arena_memory_resource final : public device_memory_resource {
/// If true, dump memory information to log on allocation failure.
bool dump_log_on_failure_{};
/// The logger for memory dump.
std::shared_ptr<spdlog::logger> logger_{};
std::shared_ptr<logger> logger_{};
/// Mutex for read and write locks on arena maps.
mutable std::shared_mutex map_mtx_;
/// Mutex for shared and unique locks on the mr.
Expand Down
Loading

0 comments on commit 4cfa31f

Please sign in to comment.