From b41d10b74e22566e2ae77e0a4f3e08a439531b87 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+ktf@users.noreply.github.com> Date: Thu, 11 Jan 2024 13:48:25 +0100 Subject: [PATCH] DPL: remove need for special engineering type We can simply use Instruments one and convert them to something sensible when using the FairLogger implementation. --- Framework/Core/src/runDataProcessing.cxx | 4 +- .../Foundation/include/Framework/Signpost.h | 48 ++++++++++++++----- Framework/Foundation/test/test_Signpost.cxx | 2 +- .../Foundation/test/test_SignpostLogger.cxx | 2 +- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/Framework/Core/src/runDataProcessing.cxx b/Framework/Core/src/runDataProcessing.cxx index e9d1f3d4ee314..325417235cd83 100644 --- a/Framework/Core/src/runDataProcessing.cxx +++ b/Framework/Core/src/runDataProcessing.cxx @@ -800,7 +800,7 @@ void processChildrenOutput(DriverInfo& driverInfo, } O2_SIGNPOST_ID_FROM_POINTER(sid, driver, &info); - O2_SIGNPOST_START(driver, sid, "bytes_processed", "bytes processed by " O2_ENG_TYPE(pid, "d"), info.pid); + O2_SIGNPOST_START(driver, sid, "bytes_processed", "bytes processed by %{xcode:pid}d", info.pid); std::string_view s = info.unprinted; size_t pos = 0; @@ -848,7 +848,7 @@ void processChildrenOutput(DriverInfo& driverInfo, size_t oldSize = info.unprinted.size(); info.unprinted = std::string(s); int64_t bytesProcessed = oldSize - info.unprinted.size(); - O2_SIGNPOST_END(driver, sid, "bytes_processed", "bytes processed by " O2_ENG_TYPE(network - size - in - bytes, PRIi64), bytesProcessed); + O2_SIGNPOST_END(driver, sid, "bytes_processed", "bytes processed by %{xcode:network-size-in-bytes}" PRIi64, bytesProcessed); } } diff --git a/Framework/Foundation/include/Framework/Signpost.h b/Framework/Foundation/include/Framework/Signpost.h index 44421687a75c9..f286e43d16afc 100644 --- a/Framework/Foundation/include/Framework/Signpost.h +++ b/Framework/Foundation/include/Framework/Signpost.h @@ -12,6 +12,7 @@ #define O2_FRAMEWORK_SIGNPOST_H_ #include +#include struct o2_log_handle_t { char const* name = nullptr; @@ -19,6 +20,31 @@ struct o2_log_handle_t { o2_log_handle_t* next = nullptr; }; +// Helper function which replaces engineering types with a printf +// compatible format string. +template +constexpr auto remove_engineering_type(char const (&src)[N]) { + std::array res = {}; + // do whatever string manipulation you want in res. + char *t = res.data(); + for (int i = 0; i < N; ++i) { + if (src[i] == '%' && src[i+1] == '{') + { + *t++ = src[i]; + while (src[i] != '}' && src[i] != 0) { + ++i; + } + if (src[i] == 0) { + *t = 0; + return res; + } + } else { + *t++ = src[i]; + } + } + return res; +} + // Loggers registry is actually a feature available to all platforms // We use this to register the loggers and to walk over them. // So that also on mac we can have a list of all the registered loggers. @@ -71,10 +97,10 @@ void* _o2_log_create(char const* name, char const* category); #define O2_LOG_DEBUG(log, ...) os_log_debug(private_o2_log_##log, __VA_ARGS__) #define O2_SIGNPOST_ID_FROM_POINTER(name, log, pointer) os_signpost_id_t name = os_signpost_id_make_with_pointer(private_o2_log_##log, pointer) #define O2_SIGNPOST_ID_GENERATE(name, log) os_signpost_id_t name = os_signpost_id_generate(private_o2_log_##log) -#define O2_SIGNPOST_EVENT_EMIT(log, id, name, ...) os_signpost_event_emit(private_o2_log_##log, id, name, __VA_ARGS__) -#define O2_SIGNPOST_START(log, id, name, ...) os_signpost_interval_begin(private_o2_log_##log, id, name, __VA_ARGS__) -#define O2_SIGNPOST_END(log, id, name, ...) os_signpost_interval_end(private_o2_log_##log, id, name, __VA_ARGS__) -#define O2_ENG_TYPE(x, what) "%{xcode:" #x "}" what +// FIXME: use __VA_OPT__ when available in C++20 +#define O2_SIGNPOST_EVENT_EMIT(log, id, name, format, ...) os_signpost_event_emit(private_o2_log_##log, id, name, format, ##__VA_ARGS__) +#define O2_SIGNPOST_START(log, id, name, format, ...) os_signpost_interval_begin(private_o2_log_##log, id, name, format, ##__VA_ARGS__) +#define O2_SIGNPOST_END(log, id, name, format, ...) os_signpost_interval_end(private_o2_log_##log, id, name, format, ##__VA_ARGS__) #ifdef O2_SIGNPOST_IMPLEMENTATION /// We use a wrapper so that we can keep track of the logs. @@ -465,10 +491,9 @@ void _o2_log_set_stacktrace(_o2_log_t* log, int stacktrace) #define O2_LOG_DEBUG(log, ...) O2_LOG_MACRO(__VA_ARGS__) #define O2_SIGNPOST_ID_FROM_POINTER(name, log, pointer) _o2_signpost_id_t name = _o2_signpost_id_make_with_pointer(private_o2_log_##log, pointer) #define O2_SIGNPOST_ID_GENERATE(name, log) _o2_signpost_id_t name = _o2_signpost_id_generate_local(private_o2_log_##log) -#define O2_SIGNPOST_EVENT_EMIT(log, id, name, ...) _o2_signpost_event_emit(private_o2_log_##log, id, name, __VA_ARGS__) -#define O2_SIGNPOST_START(log, id, name, ...) _o2_signpost_interval_begin(private_o2_log_##log, id, name, __VA_ARGS__) -#define O2_SIGNPOST_END(log, id, name, ...) _o2_signpost_interval_end(private_o2_log_##log, id, name, __VA_ARGS__) -#define O2_ENG_TYPE(x, what) "%" what +#define O2_SIGNPOST_EVENT_EMIT(log, id, name, format, ...) _o2_signpost_event_emit(private_o2_log_##log, id, name, remove_engineering_type(format).data(), ##__VA_ARGS__) +#define O2_SIGNPOST_START(log, id, name, format, ...) _o2_signpost_interval_begin(private_o2_log_##log, id, name, remove_engineering_type(format).data(), ##__VA_ARGS__) +#define O2_SIGNPOST_END(log, id, name, format, ...) _o2_signpost_interval_end(private_o2_log_##log, id, name, remove_engineering_type(format).data(), ##__VA_ARGS__) #else // This is the release implementation, it does nothing. #define O2_DECLARE_DYNAMIC_LOG(x) #define O2_DECLARE_DYNAMIC_STACKTRACE_LOG(x) @@ -478,10 +503,9 @@ void _o2_log_set_stacktrace(_o2_log_t* log, int stacktrace) #define O2_LOG_DEBUG(log, ...) #define O2_SIGNPOST_ID_FROM_POINTER(name, log, pointer) #define O2_SIGNPOST_ID_GENERATE(name, log) -#define O2_SIGNPOST_EVENT_EMIT(log, id, name, ...) -#define O2_SIGNPOST_START(log, id, name, ...) -#define O2_SIGNPOST_END(log, id, name, ...) -#define O2_ENG_TYPE(x) +#define O2_SIGNPOST_EVENT_EMIT(log, id, name, format, ...) +#define O2_SIGNPOST_START(log, id, name, format, ...) +#define O2_SIGNPOST_END(log, id, name, format, ...) #endif #endif // O2_FRAMEWORK_SIGNPOST_H_ diff --git a/Framework/Foundation/test/test_Signpost.cxx b/Framework/Foundation/test/test_Signpost.cxx index faf5a259fc6a6..24b6afaec5c3d 100644 --- a/Framework/Foundation/test/test_Signpost.cxx +++ b/Framework/Foundation/test/test_Signpost.cxx @@ -38,7 +38,7 @@ int main(int argc, char** argv) // This has an engineering type, which we will not use on Linux / FairLogger O2_SIGNPOST_ID_FROM_POINTER(id4, test_Signpost, &id3); - O2_SIGNPOST_START(test_Signpost, id4, "Test category", "A signpost with an engineering type formatter " O2_ENG_TYPE(size - in - bytes, "d"), 1); + O2_SIGNPOST_START(test_Signpost, id4, "Test category", "A signpost with an engineering type formatter %{size-in-bytes}d", 1); O2_SIGNPOST_END(test_Signpost, id4, "Test category", "A signpost interval from a pointer"); O2_SIGNPOST_START(test_SignpostDynamic, id, "Test category", "This is dynamic signpost which you will not see, because they are off by default"); diff --git a/Framework/Foundation/test/test_SignpostLogger.cxx b/Framework/Foundation/test/test_SignpostLogger.cxx index 4ce862dfb4c74..74da35abf7c70 100644 --- a/Framework/Foundation/test/test_SignpostLogger.cxx +++ b/Framework/Foundation/test/test_SignpostLogger.cxx @@ -46,7 +46,7 @@ int main(int argc, char** argv) // This has an engineering type, which we will not use on Linux / FairLogger O2_SIGNPOST_ID_FROM_POINTER(id4, test_Signpost, &id3); - O2_SIGNPOST_START(test_Signpost, id4, "Test category", "A signpost with an engineering type formatter " O2_ENG_TYPE(size - in - bytes, "d"), 1); + O2_SIGNPOST_START(test_Signpost, id4, "Test category", "A signpost with an engineering type formatter %{size-in-bytes}d", 1); O2_SIGNPOST_END(test_Signpost, id4, "Test category", "A signpost interval from a pointer"); O2_SIGNPOST_START(test_SignpostDynamic, id, "Test category", "This is dynamic signpost which you will not see, because they are off by default");