From 0f767ef605fe5642ca905bf149c1dfd8a9b519ae Mon Sep 17 00:00:00 2001 From: jatinchowdhury18 Date: Tue, 9 Apr 2024 01:45:41 -0700 Subject: [PATCH] Improvements for logging with formatting (#522) * Improvements for logging with formatting * Adding comments and tests * Apply clang-format * Ci fixes --------- Co-authored-by: github-actions[bot] --- CMakeLists.txt | 3 + .../Allocators/chowdsp_STLArenaAllocator.h | 52 +++++++++++++++ .../chowdsp_data_structures.h | 1 + .../Loggers/chowdsp_BaseLogger.cpp | 17 +++++ .../Loggers/chowdsp_BaseLogger.h | 11 ++++ .../Loggers/chowdsp_FormatHelpers.h | 65 +++++++++++++++++++ .../Loggers/chowdsp_LogFileHelpers.cpp | 4 +- .../Loggers/chowdsp_Logger.cpp | 2 +- .../chowdsp_logging/Loggers/chowdsp_Logger.h | 3 +- .../chowdsp_logging/chowdsp_logging.cpp | 1 + .../common/chowdsp_logging/chowdsp_logging.h | 4 +- .../CMakeLists.txt | 1 + .../STLArenaAllocatorTest.cpp | 27 ++++++++ .../chowdsp_logging_test/CMakeLists.txt | 1 + .../CustomFormattingTest.cpp | 21 ++++++ .../chowdsp_logging_test/PluginLoggerTest.cpp | 3 + 16 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 modules/common/chowdsp_data_structures/Allocators/chowdsp_STLArenaAllocator.h create mode 100644 modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.cpp create mode 100644 modules/common/chowdsp_logging/Loggers/chowdsp_FormatHelpers.h create mode 100644 tests/common_tests/chowdsp_data_structures_test/STLArenaAllocatorTest.cpp create mode 100644 tests/common_tests/chowdsp_logging_test/CustomFormattingTest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 345c87edb..19eb92c77 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,9 @@ else() list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake) list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/test) + # Useful for testing with address sanitizer: + # set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -g") + if(CHOWDSP_ENABLE_TESTING) enable_testing() add_subdirectory(tests) diff --git a/modules/common/chowdsp_data_structures/Allocators/chowdsp_STLArenaAllocator.h b/modules/common/chowdsp_data_structures/Allocators/chowdsp_STLArenaAllocator.h new file mode 100644 index 000000000..b41cef897 --- /dev/null +++ b/modules/common/chowdsp_data_structures/Allocators/chowdsp_STLArenaAllocator.h @@ -0,0 +1,52 @@ +#pragma once + +namespace chowdsp +{ +/** A (mostly) STL-conforming allocator backed by a memory arena of your choosing. */ +template +struct STLArenaAllocator +{ + using value_type = T; + + ArenaType& arena; + + explicit STLArenaAllocator (ArenaType& a) noexcept : arena (a) + { + } + + template + explicit STLArenaAllocator (const STLArenaAllocator& other) noexcept + : arena (other.arena) + { + } + + template + struct rebind + { + using other = STLArenaAllocator; + }; + + T* allocate (std::size_t n) + { + return static_cast (arena.allocate_bytes (n, alignof (T))); + } + + void deallocate (T*, std::size_t) const + { + // no-op... + // memory will be re-claimed when the arena is cleared. + } +}; + +template +constexpr bool operator== (const STLArenaAllocator& x, const STLArenaAllocator& y) noexcept +{ + return &x.arena == &y.arena; +} + +template +constexpr bool operator!= (const STLArenaAllocator& x, const STLArenaAllocator& y) noexcept +{ + return ! (x == y); +} +} // namespace chowdsp diff --git a/modules/common/chowdsp_data_structures/chowdsp_data_structures.h b/modules/common/chowdsp_data_structures/chowdsp_data_structures.h index c983e5e4d..b0033c746 100644 --- a/modules/common/chowdsp_data_structures/chowdsp_data_structures.h +++ b/modules/common/chowdsp_data_structures/chowdsp_data_structures.h @@ -39,6 +39,7 @@ BEGIN_JUCE_MODULE_DECLARATION #include "Allocators/chowdsp_ArenaAllocator.h" #include "Allocators/chowdsp_ChainedArenaAllocator.h" +#include "Allocators/chowdsp_STLArenaAllocator.h" #include "Structures/chowdsp_BucketArray.h" #include "Structures/chowdsp_AbstractTree.h" diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.cpp new file mode 100644 index 000000000..0fba8da2d --- /dev/null +++ b/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.cpp @@ -0,0 +1,17 @@ +#include "chowdsp_BaseLogger.h" + +namespace chowdsp +{ +BaseLogger* BaseLogger::global_logger = nullptr; + +void set_global_logger (BaseLogger* logger) +{ + BaseLogger::global_logger = logger; + juce::Logger::setCurrentLogger (logger); +} + +BaseLogger* get_global_logger() +{ + return BaseLogger::global_logger; +} +} // namespace chowdsp diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.h b/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.h index 6251d4cab..9891079ea 100644 --- a/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.h +++ b/modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.h @@ -6,6 +6,9 @@ struct BaseLogger : juce::Logger { spdlog::logger internal_logger { "chowdsp_log" }; spdlog::sink_ptr console_sink {}; + Broadcaster onLogMessage {}; + ChainedArenaAllocator arena { 8192 }; + static BaseLogger* global_logger; BaseLogger() { @@ -20,6 +23,14 @@ struct BaseLogger : juce::Logger void logMessage (const juce::String& message) override { internal_logger.info (message.toStdString()); + onLogMessage (message); + arena.clear(); } }; + +/** Set's a (static) logger to be used globally. */ +void set_global_logger (BaseLogger*); + +/** Returns the global logger, or nullptr if none has been set. */ +BaseLogger* get_global_logger(); } // namespace chowdsp diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_FormatHelpers.h b/modules/common/chowdsp_logging/Loggers/chowdsp_FormatHelpers.h new file mode 100644 index 000000000..ad7bfd6a7 --- /dev/null +++ b/modules/common/chowdsp_logging/Loggers/chowdsp_FormatHelpers.h @@ -0,0 +1,65 @@ +#pragma once + +/** Custom formatter for juce::String */ +template <> +struct fmt::formatter : formatter +{ + static auto format (const juce::String& str, format_context& ctx) -> decltype (ctx.out()) + { + return fmt::format_to (ctx.out(), "{}", str.toStdString()); + } +}; + +/** Custom formatter for std::span */ +template +struct fmt::formatter> : formatter +{ + static auto format (nonstd::span span, format_context& ctx) -> decltype (ctx.out()) + { + return fmt::format_to (ctx.out(), "{{{}}}", fmt::join (span, ",")); + } +}; + +namespace chowdsp +{ +/** Implementation of fmt::vformat using a memory arena. */ +template +std::string_view vformat (ArenaType& arena, fmt::string_view format_str, fmt::format_args args) +{ + using FormatAllocator = STLArenaAllocator; + FormatAllocator alloc { arena }; + fmt::basic_memory_buffer buffer { alloc }; + fmt::vformat_to (std::back_inserter (buffer), format_str, args); + return { buffer.data(), buffer.size() }; +} + +/** Implementation of fmt::format using a memory arena. */ +template +std::string_view format (ArenaType& arena, fmt::string_view format_str, const Args&... args) +{ + return vformat (arena, format_str, fmt::make_format_args (args...)); +} + +/** Implementation of fmt::format using the global logger's memory arena. */ +template +std::string_view format (fmt::string_view format_str, const Args&... args) +{ + auto* global_logger = get_global_logger(); + if (global_logger == nullptr) + { + // make sure you've set up the global logger before calling this! + jassertfalse; + return {}; + } + + // If this is being called from multiple threads, we might need to synchronize the arena here? + return format (global_logger->arena, format_str, std::forward (args)...); +} + +/** A formatted wrapped for juce::Logger::writeToLog(). */ +template +void log (fmt::string_view format_str, const Args&... args) +{ + juce::Logger::writeToLog (toString (format (format_str, std::forward (args)...))); +} +} // namespace chowdsp diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp index 493426e83..df4fdf50d 100644 --- a/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp +++ b/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp @@ -79,10 +79,10 @@ namespace LogFileHelpers { juce::Logger::writeToLog (toString (signal == 0 ? exitString : crashString)); - if (auto* logger = dynamic_cast (juce::Logger::getCurrentLogger())) + if (auto* logger = get_global_logger()) flushLogger (logger); - juce::Logger::setCurrentLogger (nullptr); + set_global_logger (nullptr); } static juce::File getSystemLogFileFolder() diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp index 0ace02b70..91b19e7da 100644 --- a/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp +++ b/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp @@ -30,7 +30,7 @@ Logger::Logger (const LogFileParams& loggerParams, logger.internal_logger.sinks().push_back (file_sink); logger.internal_logger.info ("Starting log file: " + log_file.getFullPathName().toStdString()); - juce::Logger::setCurrentLogger (&logger); + set_global_logger (&logger); juce::SystemStats::setApplicationCrashHandler (signalHandler); diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h b/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h index 9b515543d..c2d3317d3 100644 --- a/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h +++ b/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h @@ -16,7 +16,7 @@ class Logger : private juce::Timer const LogFileParams params; -private: +protected: void timerCallback() override; CrashLogHelpers::CrashLogAnalysisCallback crashLogAnalysisCallback = &CrashLogHelpers::defaultCrashLogAnalyzer; @@ -26,6 +26,7 @@ class Logger : private juce::Timer BaseLogger logger; +private: JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Logger) }; } // namespace chowdsp diff --git a/modules/common/chowdsp_logging/chowdsp_logging.cpp b/modules/common/chowdsp_logging/chowdsp_logging.cpp index 00a53e368..5d0c8725b 100644 --- a/modules/common/chowdsp_logging/chowdsp_logging.cpp +++ b/modules/common/chowdsp_logging/chowdsp_logging.cpp @@ -1,5 +1,6 @@ #include "chowdsp_logging.h" +#include "Loggers/chowdsp_BaseLogger.cpp" #include "Loggers/chowdsp_LogFileHelpers.cpp" #include "Loggers/chowdsp_CrashLogHelpers.cpp" #include "Loggers/chowdsp_Logger.cpp" diff --git a/modules/common/chowdsp_logging/chowdsp_logging.h b/modules/common/chowdsp_logging/chowdsp_logging.h index b05fb6eac..e4e0d2f6c 100644 --- a/modules/common/chowdsp_logging/chowdsp_logging.h +++ b/modules/common/chowdsp_logging/chowdsp_logging.h @@ -8,7 +8,7 @@ BEGIN_JUCE_MODULE_DECLARATION version: 2.2.0 name: ChowDSP Logging description: ChowDSP Logging module - dependencies: chowdsp_core, chowdsp_data_structures + dependencies: chowdsp_core, chowdsp_data_structures, chowdsp_listeners website: https://ccrma.stanford.edu/~jatin/chowdsp license: BSD 3-Clause @@ -22,6 +22,7 @@ BEGIN_JUCE_MODULE_DECLARATION #include #include +#include JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wswitch-enum", "-Wfloat-equal", @@ -40,6 +41,7 @@ JUCE_END_IGNORE_WARNINGS_GCC_LIKE #include "Loggers/chowdsp_LogFileHelpers.h" #include "Loggers/chowdsp_CrashLogHelpers.h" #include "Loggers/chowdsp_Logger.h" +#include "Loggers/chowdsp_FormatHelpers.h" // legacy #include "Loggers/chowdsp_PluginLogger.h" diff --git a/tests/common_tests/chowdsp_data_structures_test/CMakeLists.txt b/tests/common_tests/chowdsp_data_structures_test/CMakeLists.txt index 57ea56f17..9a0b126ba 100644 --- a/tests/common_tests/chowdsp_data_structures_test/CMakeLists.txt +++ b/tests/common_tests/chowdsp_data_structures_test/CMakeLists.txt @@ -16,6 +16,7 @@ target_sources(chowdsp_data_structures_test TupleHelpersTest.cpp VectorHelpersTest.cpp SmallMapTest.cpp + STLArenaAllocatorTest.cpp ) target_compile_features(chowdsp_data_structures_test PRIVATE cxx_std_20) diff --git a/tests/common_tests/chowdsp_data_structures_test/STLArenaAllocatorTest.cpp b/tests/common_tests/chowdsp_data_structures_test/STLArenaAllocatorTest.cpp new file mode 100644 index 000000000..bfeeffabd --- /dev/null +++ b/tests/common_tests/chowdsp_data_structures_test/STLArenaAllocatorTest.cpp @@ -0,0 +1,27 @@ +#include +#include + +TEST_CASE ("STL Arena Allocator Test", "[common][data-structures]") +{ + using Arena = chowdsp::ArenaAllocator<>; + Arena arena { 128 }; + + using Alloc = chowdsp::STLArenaAllocator; + Alloc alloc { arena }; + + using custom_vector = std::vector; + custom_vector vec { { 1, 2, 3, 4 }, alloc }; + REQUIRE (vec.size() == 4); + REQUIRE (vec.front() == 1); + REQUIRE (vec.back() == 4); + + vec.push_back (5); + REQUIRE (vec.size() == 5); + REQUIRE (vec.back() == 5); + + vec.erase (vec.begin()); + REQUIRE (vec.size() == 4); + vec.insert (vec.begin(), 0); + REQUIRE (vec.size() == 5); + REQUIRE (vec.front() == 0); +} diff --git a/tests/common_tests/chowdsp_logging_test/CMakeLists.txt b/tests/common_tests/chowdsp_logging_test/CMakeLists.txt index 29e727121..1d5c0fc5e 100644 --- a/tests/common_tests/chowdsp_logging_test/CMakeLists.txt +++ b/tests/common_tests/chowdsp_logging_test/CMakeLists.txt @@ -2,4 +2,5 @@ setup_catch_lib_test(chowdsp_logging_test common_tests_lib) target_sources(chowdsp_logging_test PRIVATE PluginLoggerTest.cpp + CustomFormattingTest.cpp ) diff --git a/tests/common_tests/chowdsp_logging_test/CustomFormattingTest.cpp b/tests/common_tests/chowdsp_logging_test/CustomFormattingTest.cpp new file mode 100644 index 000000000..95362b181 --- /dev/null +++ b/tests/common_tests/chowdsp_logging_test/CustomFormattingTest.cpp @@ -0,0 +1,21 @@ +#include +#include + +TEST_CASE ("Custom Formatting Test", "[common][logs]") +{ + chowdsp::ArenaAllocator<> arena { 2048 }; + + SECTION ("juce::String") + { + juce::String str { "This is a JUCE string!" }; + const auto format_result = chowdsp::format (arena, "{}", str); + REQUIRE (str == chowdsp::toString (format_result)); + } + + SECTION ("std::span") + { + std::vector vec { 0.0f, 1.0f, 2.0f, 3.0f, 4.0f }; + const auto format_result = chowdsp::format (arena, "{}", nonstd::span { vec }); + REQUIRE (format_result == "{0,1,2,3,4}"); + } +} diff --git a/tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp b/tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp index 96d663a8c..5539f2225 100644 --- a/tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp +++ b/tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp @@ -26,6 +26,7 @@ TEMPLATE_TEST_CASE ("Plugin Logger Test", "[common][logs]", chowdsp::PluginLogge { Logger logger { logFileSubDir, logFileNameRoot }; juce::Logger::writeToLog (testLogString); + chowdsp::log ("The magic number is: {}", 18); logFile = logger.getLogFile(); } @@ -34,6 +35,8 @@ TEMPLATE_TEST_CASE ("Plugin Logger Test", "[common][logs]", chowdsp::PluginLogge auto logString = logFile.loadFileAsString(); REQUIRE_MESSAGE (logString.contains (testLogString), "Test log string was not found in the log file!"); + if (std::is_same_v) + REQUIRE_MESSAGE (logString.contains ("The magic number is: 18"), "Test log string was not found in the log file!"); REQUIRE_MESSAGE (! logString.contains (testNonLogString), "Test non-log string WAS found in the log file!"); }