From e96dc383e148b40397519bec07da43b1f999f2aa Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Wed, 20 May 2026 13:00:01 -0500 Subject: [PATCH 1/9] feat: add Perfetto performance tracing support Adds optional Perfetto SDK integration (USE_PERFETTO=ON) that enables real-time per-thread tracing of JANA2 factory execution for identifying performance bottlenecks and inter-factory blocking in multithreaded runs. Changes: - CMakeLists.txt: new USE_PERFETTO option; downloads Perfetto SDK v55.3 amalgamation from GitHub releases; builds perfetto_sdk static library; sets JANA2_HAVE_PERFETTO for conditional compilation - JVersion.h.in: adds JANA2_HAVE_PERFETTO capability flag and HasPerfetto() - JPerfettoService: new JService managing the Perfetto tracing session; initializes in-process backend, writes trace file on shutdown; exposes RegisterCurrentThread(worker_id) for named thread tracks - JFactory.cc: TRACE_EVENT_BEGIN/END around Process() calls (category 'factory') - JExecutionEngine.cc: TRACE_EVENT_BEGIN/END around arrow->Fire() calls (category 'jana'); RegisterCurrentThread() at worker startup - perfetto plugin: loads JPerfettoService via -Pplugins=perfetto Usage: cmake -DUSE_PERFETTO=ON ... eicrecon -Pplugins=perfetto -Pjana:perfetto_output=trace.perfetto ... # Open trace.perfetto at https://ui.perfetto.dev Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CMakeLists.txt | 39 ++++++++++ src/libraries/JANA/CMakeLists.txt | 16 ++++ src/libraries/JANA/Engine/JExecutionEngine.cc | 15 ++++ src/libraries/JANA/JFactory.cc | 19 +++++ src/libraries/JANA/JVersion.h.in | 2 + .../JANA/Services/JPerfettoService.cc | 77 +++++++++++++++++++ .../JANA/Services/JPerfettoService.h | 36 +++++++++ src/plugins/CMakeLists.txt | 1 + src/plugins/perfetto/CMakeLists.txt | 7 ++ src/plugins/perfetto/perfetto_plugin.cc | 17 ++++ 10 files changed, 229 insertions(+) create mode 100644 src/libraries/JANA/Services/JPerfettoService.cc create mode 100644 src/libraries/JANA/Services/JPerfettoService.h create mode 100644 src/plugins/perfetto/CMakeLists.txt create mode 100644 src/plugins/perfetto/perfetto_plugin.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 955836b76..e5d4dfe2e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -68,6 +68,7 @@ option(USE_ASAN "Compile with address sanitizer" OFF) option(USE_TSAN "Compile with thread sanitizer" OFF) option(USE_CUDA "Compile CUDA-involved examples (Needed for examples/SubeventCUDAExample)." OFF) option(USE_PODIO "Compile with PODIO support" OFF) +option(USE_PERFETTO "Include Perfetto tracing SDK for performance profiling." OFF) option(BUILD_SHARED_LIBS "Build into both shared and static libs." ON) option(BUILD_EXAMPLES "Build and install examples" ON) option(BUILD_TESTS "Build and install tests" ON) @@ -134,6 +135,39 @@ if (${USE_CUDA}) find_package(CUDA REQUIRED) endif() +if (USE_PERFETTO) + set(PERFETTO_VERSION "55.3" CACHE STRING "Perfetto SDK version to download") + set(PERFETTO_SDK_URL "https://github.com/google/perfetto/releases/download/v${PERFETTO_VERSION}/perfetto-cpp-sdk-src.zip") + set(PERFETTO_SDK_ZIP "${CMAKE_BINARY_DIR}/perfetto-cpp-sdk-src-${PERFETTO_VERSION}.zip") + set(PERFETTO_SDK_DIR "${CMAKE_BINARY_DIR}/perfetto_sdk_${PERFETTO_VERSION}") + + if(NOT EXISTS "${PERFETTO_SDK_DIR}/perfetto.cc") + message(STATUS "Downloading Perfetto SDK v${PERFETTO_VERSION}...") + file(DOWNLOAD "${PERFETTO_SDK_URL}" "${PERFETTO_SDK_ZIP}" + STATUS DOWNLOAD_STATUS + TLS_VERIFY ON + ) + list(GET DOWNLOAD_STATUS 0 DOWNLOAD_ERROR_CODE) + if(DOWNLOAD_ERROR_CODE) + list(GET DOWNLOAD_STATUS 1 DOWNLOAD_ERROR_MSG) + message(FATAL_ERROR "Failed to download Perfetto SDK from ${PERFETTO_SDK_URL}: ${DOWNLOAD_ERROR_MSG}") + endif() + file(ARCHIVE_EXTRACT INPUT "${PERFETTO_SDK_ZIP}" DESTINATION "${PERFETTO_SDK_DIR}") + endif() + + add_library(perfetto_sdk STATIC ${PERFETTO_SDK_DIR}/perfetto.cc) + target_include_directories(perfetto_sdk PUBLIC + $ + $ + ) + target_compile_options(perfetto_sdk PRIVATE -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers) + install(TARGETS perfetto_sdk EXPORT jana2_targets DESTINATION lib) + install(FILES ${PERFETTO_SDK_DIR}/perfetto.h DESTINATION include/perfetto) + set(JANA2_HAVE_PERFETTO 1) +else() + set(JANA2_HAVE_PERFETTO 0) +endif() + #--------- # Report back to the user what we've discovered @@ -178,6 +212,11 @@ if (${USE_CUDA}) else() message(STATUS "USE_CUDA Off") endif() +if (USE_PERFETTO) + message(STATUS "USE_PERFETTO On") +else() + message(STATUS "USE_PERFETTO Off") +endif() if (${USE_PODIO}) message(STATUS "USE_PODIO On --> " ${podio_DIR}) else() diff --git a/src/libraries/JANA/CMakeLists.txt b/src/libraries/JANA/CMakeLists.txt index 27a03b9a5..7ecc67237 100644 --- a/src/libraries/JANA/CMakeLists.txt +++ b/src/libraries/JANA/CMakeLists.txt @@ -66,6 +66,10 @@ if (NOT ${USE_XERCES}) message(STATUS "Skipping support for libJANA's JGeometryXML because USE_XERCES=Off") endif() +if (USE_PERFETTO) + list(APPEND JANA2_SOURCES Services/JPerfettoService.cc) +endif() + add_library(jana2 OBJECT ${JANA2_SOURCES}) find_package(Threads REQUIRED) @@ -74,6 +78,10 @@ target_link_libraries(jana2 PUBLIC ${CMAKE_DL_LIBS} Threads::Threads) target_link_libraries(jana2 PRIVATE VendoredTomlPlusPlus) target_link_libraries(jana2 PRIVATE VendoredMD5) # To pull in the header file +if (USE_PERFETTO) + target_link_libraries(jana2 PUBLIC perfetto_sdk) +endif() + if (${USE_PODIO}) target_link_libraries(jana2 PUBLIC podio::podio podio::podioRootIO) elseif (${USE_ROOT}) @@ -89,6 +97,10 @@ target_include_directories(jana2_static_lib PUBLIC $) target_link_libraries(jana2_static_lib PUBLIC ${CMAKE_DL_LIBS} Threads::Threads) target_link_libraries(jana2_static_lib PUBLIC VendoredTomlPlusPlus) +if (USE_PERFETTO) + target_link_libraries(jana2_static_lib PUBLIC perfetto_sdk) +endif() + if (${USE_PODIO}) target_link_libraries(jana2_static_lib PUBLIC podio::podio podio::podioRootIO) elseif (${USE_ROOT}) @@ -105,6 +117,10 @@ target_include_directories(jana2_shared_lib PUBLIC $) target_link_libraries(jana2_shared_lib PUBLIC ${CMAKE_DL_LIBS} Threads::Threads) target_link_libraries(jana2_shared_lib PUBLIC VendoredTomlPlusPlus) +if (USE_PERFETTO) + target_link_libraries(jana2_shared_lib PUBLIC perfetto_sdk) +endif() + if (${USE_PODIO}) target_link_libraries(jana2_shared_lib PUBLIC podio::podio podio::podioRootIO) elseif (${USE_ROOT}) diff --git a/src/libraries/JANA/Engine/JExecutionEngine.cc b/src/libraries/JANA/Engine/JExecutionEngine.cc index ebcf89448..32d4038dd 100644 --- a/src/libraries/JANA/Engine/JExecutionEngine.cc +++ b/src/libraries/JANA/Engine/JExecutionEngine.cc @@ -1,6 +1,11 @@ #include "JExecutionEngine.h" #include +#include + +#if JANA2_HAVE_PERFETTO +#include +#endif #include #include @@ -416,12 +421,22 @@ void JExecutionEngine::RunWorker(Worker worker) { LOG_DEBUG(GetLogger()) << "Launched worker thread " << worker.worker_id << LOG_END; jana2_worker_id = worker.worker_id; jana2_worker_backtrace = worker.backtrace; +#if JANA2_HAVE_PERFETTO + JPerfettoService::RegisterCurrentThread(worker.worker_id); +#endif try { Task task; while (true) { ExchangeTask(task, worker.worker_id); if (task.arrow == nullptr) break; // Exit as soon as ExchangeTask() stops blocking +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("jana", perfetto::DynamicString{task.arrow->GetName()}, + "worker_id", (uint64_t)worker.worker_id); +#endif task.arrow->Fire(task.input_event, task.outputs, task.output_count, task.status); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("jana"); +#endif } LOG_DEBUG(GetLogger()) << "Stopped worker thread " << worker.worker_id << LOG_END; } diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 685e23006..eed88c394 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -6,6 +6,11 @@ #include #include #include +#include + +#if JANA2_HAVE_PERFETTO +#include +#endif class FlagGuard { @@ -152,10 +157,24 @@ void JFactory::Create(const JEvent& event) { input->Populate(event); } if (m_callback_style == CallbackStyle::LegacyMode) { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("factory", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "event_nr", event.GetEventNumber()); +#endif CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event.shared_from_this()); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory"); +#endif } else if (m_callback_style == CallbackStyle::ExpertMode) { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("factory", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "event_nr", event.GetEventNumber()); +#endif CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory"); +#endif } else { throw JException("Invalid callback style"); diff --git a/src/libraries/JANA/JVersion.h.in b/src/libraries/JANA/JVersion.h.in index ac495c47f..db710b399 100644 --- a/src/libraries/JANA/JVersion.h.in +++ b/src/libraries/JANA/JVersion.h.in @@ -9,6 +9,7 @@ #define JANA2_HAVE_PODIO @JANA2_HAVE_PODIO@ #define JANA2_HAVE_ROOT @JANA2_HAVE_ROOT@ #define JANA2_HAVE_XERCES @JANA2_HAVE_XERCES@ +#define JANA2_HAVE_PERFETTO @JANA2_HAVE_PERFETTO@ #define JANA2_COMMIT_HASH "@JVERSION_COMMIT_HASH@" #define JANA2_COMMIT_DATE "@JVERSION_COMMIT_DATE@" @@ -36,6 +37,7 @@ struct JVersion { static constexpr bool HasPodio() { return JANA2_HAVE_PODIO; } static constexpr bool HasROOT() { return JANA2_HAVE_ROOT; } static constexpr bool HasXerces() { return JANA2_HAVE_XERCES; } + static constexpr bool HasPerfetto() { return JANA2_HAVE_PERFETTO; } static std::string GetVersion(); static constexpr uint64_t GetVersionNumber(); diff --git a/src/libraries/JANA/Services/JPerfettoService.cc b/src/libraries/JANA/Services/JPerfettoService.cc new file mode 100644 index 000000000..f2cb4ba2e --- /dev/null +++ b/src/libraries/JANA/Services/JPerfettoService.cc @@ -0,0 +1,77 @@ +// Copyright 2026, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#include + +#if JANA2_HAVE_PERFETTO + +#include +#include + +// Allocate static storage for the categories (must be in exactly one .cc file). +PERFETTO_TRACK_EVENT_STATIC_STORAGE(); + +void JPerfettoService::Init() { + auto params = GetApplication()->GetJParameterManager(); + params->SetDefaultParameter("jana:perfetto_output", m_output_file, + "Output file path for the Perfetto trace (written on exit)."); + params->SetDefaultParameter("jana:perfetto_enabled", m_enabled, + "Enable Perfetto tracing."); + + if (!m_enabled) return; + + perfetto::TracingInitArgs args; + args.backends |= perfetto::kInProcessBackend; + perfetto::Tracing::Initialize(args); + perfetto::TrackEvent::Register(); + + perfetto::TraceConfig cfg; + cfg.add_buffers()->set_size_kb(1024 * 256); // 256 MB ring buffer + auto* ds_cfg = cfg.add_data_sources()->mutable_config(); + ds_cfg->set_name("track_event"); + + m_tracing_session = perfetto::Tracing::NewTrace(); + m_tracing_session->Setup(cfg); + m_tracing_session->StartBlocking(); + + LOG_INFO(GetLogger()) << "Perfetto tracing started. Output will be written to: " << m_output_file << LOG_END; +} + +JPerfettoService::~JPerfettoService() { + if (!m_enabled || !m_tracing_session) return; + + perfetto::TrackEvent::Flush(); + m_tracing_session->StopBlocking(); + + std::vector trace_data(m_tracing_session->ReadTraceBlocking()); + std::ofstream output(m_output_file, std::ios::out | std::ios::binary); + if (!output) { + std::cerr << "JPerfettoService: Failed to open Perfetto output file: " << m_output_file << "\n"; + return; + } + output.write(trace_data.data(), static_cast(trace_data.size())); + std::cout << "JPerfettoService: Perfetto trace written to: " << m_output_file + << " (" << trace_data.size() / 1024 << " kB)\n"; + std::cout << "JPerfettoService: Open the trace at https://ui.perfetto.dev\n"; +} + +void JPerfettoService::RegisterCurrentThread(int worker_id) { + std::ostringstream name; + name << "Worker " << worker_id; + perfetto::ThreadTrack track = perfetto::ThreadTrack::Current(); + perfetto::protos::gen::TrackDescriptor desc = track.Serialize(); + desc.set_name(name.str()); + perfetto::TrackEvent::SetTrackDescriptor(track, desc); +} + +#else // JANA2_HAVE_PERFETTO + +void JPerfettoService::Init() { + std::cerr << "JPerfettoService: JANA2 was not built with Perfetto support (USE_PERFETTO=OFF).\n"; +} + +JPerfettoService::~JPerfettoService() {} + +void JPerfettoService::RegisterCurrentThread(int /*worker_id*/) {} + +#endif // JANA2_HAVE_PERFETTO diff --git a/src/libraries/JANA/Services/JPerfettoService.h b/src/libraries/JANA/Services/JPerfettoService.h new file mode 100644 index 000000000..87988ff03 --- /dev/null +++ b/src/libraries/JANA/Services/JPerfettoService.h @@ -0,0 +1,36 @@ +// Copyright 2026, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#pragma once + +#include +#include + +#if JANA2_HAVE_PERFETTO +#include + +// Declare categories in a header so all TUs that use TRACE_EVENT share the same registry. +PERFETTO_DEFINE_CATEGORIES( + perfetto::Category("jana").SetDescription("JANA2 framework events (arrow dispatch)"), + perfetto::Category("factory").SetDescription("JANA2 factory execution per event") +); +#endif // JANA2_HAVE_PERFETTO + +class JPerfettoService : public JService { + +public: + JPerfettoService() = default; + ~JPerfettoService() override; + + void Init() override; + + /// Call from each worker thread at startup to register the thread track with its worker ID. + static void RegisterCurrentThread(int worker_id); + +private: +#if JANA2_HAVE_PERFETTO + std::unique_ptr m_tracing_session; +#endif + std::string m_output_file = "jana_trace.perfetto"; + bool m_enabled = true; +}; diff --git a/src/plugins/CMakeLists.txt b/src/plugins/CMakeLists.txt index ac7680fc8..f3db8d855 100644 --- a/src/plugins/CMakeLists.txt +++ b/src/plugins/CMakeLists.txt @@ -6,4 +6,5 @@ add_subdirectory(janarate) add_subdirectory(janaroot) add_subdirectory(janaview) add_subdirectory(regressiontest) +add_subdirectory(perfetto) diff --git a/src/plugins/perfetto/CMakeLists.txt b/src/plugins/perfetto/CMakeLists.txt new file mode 100644 index 000000000..ea84d4362 --- /dev/null +++ b/src/plugins/perfetto/CMakeLists.txt @@ -0,0 +1,7 @@ +if (USE_PERFETTO) + add_jana_plugin(perfetto SOURCES perfetto_plugin.cc) + add_test(NAME jana-plugin-perfetto-tests + COMMAND jana -Pplugins=JTest,perfetto -Pjana:nevents=10) +else() + message(STATUS "Skipping plugins/perfetto because USE_PERFETTO=Off") +endif() diff --git a/src/plugins/perfetto/perfetto_plugin.cc b/src/plugins/perfetto/perfetto_plugin.cc new file mode 100644 index 000000000..bd85b9d21 --- /dev/null +++ b/src/plugins/perfetto/perfetto_plugin.cc @@ -0,0 +1,17 @@ +// Copyright 2026, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#include +#include +#include + +extern "C" { +void InitPlugin(JApplication* app) { + InitJANAPlugin(app); + app->ProvideService(std::make_shared()); + + // Ensure that the call graph is recorded so that factory dependency chains + // are captured alongside the Perfetto timeline. + app->SetParameterValue("record_call_stack", 1); +} +} // extern "C" From 994a0d82fa865b1638f8f3bcdadb7dc1060c0171 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Wed, 20 May 2026 15:51:03 -0500 Subject: [PATCH 2/9] feat: trace factory Init/ChangeRun/BeginRun/EndRun phases separately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new 'factory_init' Perfetto category to distinguish factory lifecycle callbacks from per-event Process() execution: - factory_init: Init(), ChangeRun(), BeginRun(), EndRun() callbacks - factory: Process() per event (existing) Each factory_init span carries a 'phase' annotation ('Init', 'ChangeRun', 'BeginRun', 'EndRun') and a 'run_nr' annotation on run-boundary spans. The factory name is used as the slice name in both categories, making the timeline immediately readable. The new category doubles the trace size on a typical EICrecon run (349 kB → 797 kB) because every factory initializes on first use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/libraries/JANA/JFactory.cc | 36 +++++++++++++++++++ .../JANA/Services/JPerfettoService.h | 3 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index eed88c394..d6b6ac9c6 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -140,13 +140,39 @@ void JFactory::Create(const JEvent& event) { if (mPreviousRunNumber != run_number) { if (m_callback_style == CallbackStyle::LegacyMode) { if (mPreviousRunNumber != -1) { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "EndRun"); +#endif CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory_init"); +#endif } +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); +#endif CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory_init"); + TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "BeginRun"); +#endif CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory_init"); +#endif } else if (m_callback_style == CallbackStyle::ExpertMode) { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); +#endif CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory_init"); +#endif } mPreviousRunNumber = run_number; } @@ -220,10 +246,20 @@ void JFactory::DoInit() { service->Fetch(m_app); } try { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "phase", "Init"); +#endif CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory_init"); +#endif mInitStatus = InitStatus::InitRun; } catch (...) { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT_END("factory_init"); +#endif mInitStatus = InitStatus::InitExcepted; mException = std::current_exception(); throw; diff --git a/src/libraries/JANA/Services/JPerfettoService.h b/src/libraries/JANA/Services/JPerfettoService.h index 87988ff03..f6823b9ca 100644 --- a/src/libraries/JANA/Services/JPerfettoService.h +++ b/src/libraries/JANA/Services/JPerfettoService.h @@ -12,7 +12,8 @@ // Declare categories in a header so all TUs that use TRACE_EVENT share the same registry. PERFETTO_DEFINE_CATEGORIES( perfetto::Category("jana").SetDescription("JANA2 framework events (arrow dispatch)"), - perfetto::Category("factory").SetDescription("JANA2 factory execution per event") + perfetto::Category("factory").SetDescription("JANA2 factory Process() execution per event"), + perfetto::Category("factory_init").SetDescription("JANA2 factory Init/ChangeRun/BeginRun/EndRun callbacks") ); #endif // JANA2_HAVE_PERFETTO From 3aa46b74a098cd4ba04abac68b6994a7d52170e7 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Wed, 20 May 2026 17:21:39 -0500 Subject: [PATCH 3/9] fix: use scoped TRACE_EVENT (RAII) to prevent unclosed spans on exceptions Replace all TRACE_EVENT_BEGIN/END pairs with the scoped TRACE_EVENT macro, which uses RAII to call TRACE_EVENT_END in its destructor. This ensures trace spans are always properly closed even when factory callbacks (Process, Init, ChangeRun, BeginRun, EndRun) or arrow Fire() throw exceptions. Previously, factories that threw (e.g. 'No beam protons found') would appear as 'did not end' in the Perfetto UI because the exception propagated past the TRACE_EVENT_END call to the outer catch block. Changes: - JFactory.cc: DoInit(), Create() Process/EndRun/ChangeRun/BeginRun all wrapped in explicit {} blocks with scoped TRACE_EVENT - JExecutionEngine.cc: arrow->Fire() wrapped in explicit {} block - Remove now-redundant manual TRACE_EVENT_END from DoInit catch block Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/libraries/JANA/Engine/JExecutionEngine.cc | 11 ++- src/libraries/JANA/JFactory.cc | 80 ++++++++----------- 2 files changed, 37 insertions(+), 54 deletions(-) diff --git a/src/libraries/JANA/Engine/JExecutionEngine.cc b/src/libraries/JANA/Engine/JExecutionEngine.cc index 32d4038dd..be3669cf2 100644 --- a/src/libraries/JANA/Engine/JExecutionEngine.cc +++ b/src/libraries/JANA/Engine/JExecutionEngine.cc @@ -429,14 +429,13 @@ void JExecutionEngine::RunWorker(Worker worker) { while (true) { ExchangeTask(task, worker.worker_id); if (task.arrow == nullptr) break; // Exit as soon as ExchangeTask() stops blocking + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("jana", perfetto::DynamicString{task.arrow->GetName()}, - "worker_id", (uint64_t)worker.worker_id); -#endif - task.arrow->Fire(task.input_event, task.outputs, task.output_count, task.status); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("jana"); + TRACE_EVENT("jana", perfetto::DynamicString{task.arrow->GetName()}, + "worker_id", (uint64_t)worker.worker_id); #endif + task.arrow->Fire(task.input_event, task.outputs, task.output_count, task.status); + } } LOG_DEBUG(GetLogger()) << "Stopped worker thread " << worker.worker_id << LOG_END; } diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index d6b6ac9c6..76c4fb0f3 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -141,38 +141,34 @@ void JFactory::Create(const JEvent& event) { if (m_callback_style == CallbackStyle::LegacyMode) { if (mPreviousRunNumber != -1) { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, "tag", GetTag(), "run_nr", run_number, "phase", "EndRun"); #endif CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory_init"); -#endif } + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); -#endif - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory_init"); - TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "BeginRun"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); #endif - CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); + } + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory_init"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "BeginRun"); #endif + CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); + } } else if (m_callback_style == CallbackStyle::ExpertMode) { + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); -#endif - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory_init"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); #endif + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); + } } mPreviousRunNumber = run_number; } @@ -182,28 +178,20 @@ void JFactory::Create(const JEvent& event) { for (auto* input : GetVariadicInputs()) { input->Populate(event); } - if (m_callback_style == CallbackStyle::LegacyMode) { -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("factory", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "event_nr", event.GetEventNumber()); -#endif - CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event.shared_from_this()); }); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory"); -#endif - } - else if (m_callback_style == CallbackStyle::ExpertMode) { + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("factory", perfetto::DynamicString{GetFactoryName()}, + TRACE_EVENT("factory", perfetto::DynamicString{GetFactoryName()}, "tag", GetTag(), "event_nr", event.GetEventNumber()); #endif - CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event); }); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory"); -#endif - } - else { - throw JException("Invalid callback style"); + if (m_callback_style == CallbackStyle::LegacyMode) { + CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event.shared_from_this()); }); + } + else if (m_callback_style == CallbackStyle::ExpertMode) { + CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event); }); + } + else { + throw JException("Invalid callback style"); + } } } catch (...) { @@ -246,20 +234,16 @@ void JFactory::DoInit() { service->Fetch(m_app); } try { + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT_BEGIN("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "phase", "Init"); -#endif - CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory_init"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "phase", "Init"); #endif + CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); + } mInitStatus = InitStatus::InitRun; } catch (...) { -#if JANA2_HAVE_PERFETTO - TRACE_EVENT_END("factory_init"); -#endif mInitStatus = InitStatus::InitExcepted; mException = std::current_exception(); throw; From 2ae0de2a803033e3a75b14e857e4bc898b4a0a2f Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Wed, 20 May 2026 18:47:29 -0500 Subject: [PATCH 4/9] fix: restructure factory span to wrap entire activation including Init Previously the factory TRACE_EVENT only covered Process(), making Init and run callback (BeginRun/ChangeRun/EndRun) spans appear as siblings before the factory span rather than children inside it. Now the factory span opens before DoInit(), so all factory_init category spans (Init, ChangeRun, BeginRun, EndRun) appear as visible children inside the factory span in the Perfetto UI. Clicking any factory span will show the Init phase as a nested child span with its own duration and phase annotation. SQL verification on a 10-event trace confirms: factory -> factory_init: 3372 (all Init/run callback spans are children) factory -> factory: 2320 (dependent factory spans as before) jana -> factory: 880 (top-level factory activations from engine) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/libraries/JANA/JFactory.cc | 165 +++++++++++++++++---------------- 1 file changed, 86 insertions(+), 79 deletions(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 76c4fb0f3..70074f2fe 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -111,78 +111,85 @@ void JFactory::Create(const JEvent& event) { std::rethrow_exception(mException); } - // Make sure Init() ran, which might except... - try { - DoInit(); // This checks mInitStatus internally before calling Init() - } - catch(...) { - // If Init() excepts, we still need to store an empty collection - mStatus = Status::Excepted; - for (auto* output : GetOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + // The factory span wraps the entire activation: Init + run callbacks + input population + Process. + // factory_init spans (Init, BeginRun, ChangeRun, EndRun) appear as children of this factory span, + // making the phase structure visible when inspecting any factory span in the trace. + { +#if JANA2_HAVE_PERFETTO + TRACE_EVENT("factory", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "event_nr", event.GetEventNumber()); +#endif + + // Make sure Init() ran, which might except... + try { + DoInit(); // This checks mInitStatus internally before calling Init() } - for (auto* output : GetVariadicOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + catch(...) { + // If Init() excepts, we still need to store an empty collection + mStatus = Status::Excepted; + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + std::rethrow_exception(mException); } - std::rethrow_exception(mException); - } - // At this point, Init() has run and has _not_ excepted + // At this point, Init() has run and has _not_ excepted - if (mStatus == Status::Excepted) { - // But Process() might have already excepted! - std::rethrow_exception(mException); - } - else if (mStatus == Status::Empty) { - // Now we know that we need to run Process() to create the data in the first place - try { - auto run_number = event.GetRunNumber(); - if (mPreviousRunNumber != run_number) { - if (m_callback_style == CallbackStyle::LegacyMode) { - if (mPreviousRunNumber != -1) { + if (mStatus == Status::Excepted) { + // But Process() might have already excepted! + std::rethrow_exception(mException); + } + else if (mStatus == Status::Empty) { + // Now we know that we need to run Process() to create the data in the first place + try { + auto run_number = event.GetRunNumber(); + if (mPreviousRunNumber != run_number) { + if (m_callback_style == CallbackStyle::LegacyMode) { + if (mPreviousRunNumber != -1) { + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "EndRun"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "EndRun"); #endif - CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); - } - { + CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); + } + } + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); #endif - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); - } - { + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); + } + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "BeginRun"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "BeginRun"); #endif - CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); + CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); + } } - } - else if (m_callback_style == CallbackStyle::ExpertMode) { - { + else if (m_callback_style == CallbackStyle::ExpertMode) { + { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); + TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); #endif - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); + } } + mPreviousRunNumber = run_number; } - mPreviousRunNumber = run_number; - } - for (auto* input : GetInputs()) { - input->Populate(event); - } - for (auto* input : GetVariadicInputs()) { - input->Populate(event); - } - { -#if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "event_nr", event.GetEventNumber()); -#endif + for (auto* input : GetInputs()) { + input->Populate(event); + } + for (auto* input : GetVariadicInputs()) { + input->Populate(event); + } + // Process() runs inside the factory span; dependent factory spans appear as children if (m_callback_style == CallbackStyle::LegacyMode) { CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event.shared_from_this()); }); } @@ -193,34 +200,34 @@ void JFactory::Create(const JEvent& event) { throw JException("Invalid callback style"); } } - } - catch (...) { - // Save everything already created even if we throw an exception - // This is so that we leave everything in a valid state just in case someone tries to catch the exception recover, - // such as EICrecon. (Remember that a missing collection in the podio frame will segfault if anyone tries to write that frame) - // Note that the collections themselves won't know that they exited early + catch (...) { + // Save everything already created even if we throw an exception + // This is so that we leave everything in a valid state just in case someone tries to catch the exception recover, + // such as EICrecon. (Remember that a missing collection in the podio frame will segfault if anyone tries to write that frame) + // Note that the collections themselves won't know that they exited early + + LOG << "Exception in JFactory::Create, prefix=" << GetPrefix(); + mStatus = Status::Excepted; + mException = std::current_exception(); + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + throw; + } - LOG << "Exception in JFactory::Create, prefix=" << GetPrefix(); - mStatus = Status::Excepted; - mException = std::current_exception(); + // Save the (successfully processed) data + mStatus = Status::Processed; for (auto* output : GetOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); } for (auto* output : GetVariadicOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); } - throw; } - - // Save the (successfully processed) data - mStatus = Status::Processed; - for (auto* output : GetOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); - } - for (auto* output : GetVariadicOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); - } - } + } // end factory span } void JFactory::DoInit() { From 7f415f9370f9bddc70db38d1c0bd40a52451683c Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Wed, 20 May 2026 19:24:22 -0500 Subject: [PATCH 5/9] feat: add inter-factory dependency flow arrows to Perfetto trace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When factory B is triggered from within factory A's Process(), Perfetto now draws an arrow from the point in A's span where B was triggered to the start of B's span, visualizing the data-dependency graph. Implementation: - Thread-local g_current_executing_factory tracks which factory span is currently open on each thread (set on span open, restored on close via RAII CallerGuard). - When B's Create() is entered and a caller factory is recorded, compute a unique FNV-1a-inspired flow ID from (caller, callee, event_number). - Emit a TRACE_EVENT_INSTANT with Flow::Global(flow_id) while still executing within A's open span — this is the arrow's tail. - Open B's span with TRACE_EVENT_BEGIN + TerminatingFlow::Global(flow_id) — this is the arrow's head. Using TRACE_EVENT_BEGIN instead of the scoped TRACE_EVENT macro avoids the if-else early-close problem (the scoped TRACE_EVENT in an if-else branch would close before DoInit/Process). - A SpanGuard RAII struct calls TRACE_EVENT_END on destruction, ensuring the span is always closed even on exception. SQL verification on a 10-event trace: 2320 flow-start instants (= factory spans that have a parent factory) Top edge: CalorimeterClusterShape → CalorimeterClusterRecoCoG (240 times) Full dependency graph visible: CKFTracking → AmbiguitySolver → ActsToTracks → TrackerHitReconstruction → SiliconTrackerDigi etc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/libraries/JANA/JFactory.cc | 41 ++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 70074f2fe..f9ab5451b 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -10,6 +10,12 @@ #if JANA2_HAVE_PERFETTO #include + +// Thread-local pointer to the factory whose span is currently open on this thread. +// Used to build inter-factory dependency flow arrows: when factory B is triggered from +// within factory A's Process(), g_current_executing_factory == A, so we can draw +// an arrow A → B in the Perfetto timeline. +static thread_local JFactory* g_current_executing_factory = nullptr; #endif @@ -114,10 +120,41 @@ void JFactory::Create(const JEvent& event) { // The factory span wraps the entire activation: Init + run callbacks + input population + Process. // factory_init spans (Init, BeginRun, ChangeRun, EndRun) appear as children of this factory span, // making the phase structure visible when inspecting any factory span in the trace. + // Flow events connect each factory span to the parent factory that triggered it, + // drawn as arrows showing the data-dependency chain in the Perfetto UI. { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "event_nr", event.GetEventNumber()); + // If a parent factory is currently executing on this thread it triggered us. + // Emit a flow-start instant inside the parent's still-open span, then open + // our span with a TerminatingFlow — Perfetto draws an arrow parent → us. + JFactory* const caller = g_current_executing_factory; + if (caller != nullptr) { + // FNV-1a-inspired hash: unique 64-bit flow ID per (caller, callee, event). + uint64_t flow_id = 14695981039346656037ULL; + flow_id = (flow_id ^ reinterpret_cast(caller)) * 1099511628211ULL; + flow_id = (flow_id ^ reinterpret_cast(this)) * 1099511628211ULL; + flow_id = (flow_id ^ static_cast(event.GetEventNumber())) * 1099511628211ULL; + // Flow starts here — we are still executing inside the parent's open span. + TRACE_EVENT_INSTANT("factory", perfetto::DynamicString{GetFactoryName()}, + perfetto::ThreadTrack::Current(), perfetto::Flow::Global(flow_id)); + // Our span starts, consuming the arrow from the parent's instant above. + TRACE_EVENT_BEGIN("factory", perfetto::DynamicString{GetFactoryName()}, + perfetto::TerminatingFlow::Global(flow_id), + "tag", GetTag(), "event_nr", event.GetEventNumber()); + } else { + TRACE_EVENT_BEGIN("factory", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "event_nr", event.GetEventNumber()); + } + // RAII: close the Perfetto span on scope exit (exception-safe). + struct SpanGuard { ~SpanGuard() { TRACE_EVENT_END("factory"); } } span_guard; + // RAII: track the currently-executing factory so sub-factories can link back to us. + // Declared after span_guard — its destructor runs first, restoring the caller + // before the span closes. + struct CallerGuard { + JFactory** slot; JFactory* saved; + ~CallerGuard() { *slot = saved; } + } caller_guard{&g_current_executing_factory, caller}; + g_current_executing_factory = this; #endif // Make sure Init() ran, which might except... From c0bcfc5538efbfaa19cc275ada863156e55dfbfd Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 21 May 2026 09:58:25 -0500 Subject: [PATCH 6/9] perf: split factory_init category into factory_init/begin_run/change_run/end_run Use separate Perfetto categories for each factory lifecycle callback so they appear as distinct tracks in the trace, instead of all being labeled factory_init: - factory_init : Init() callback (first activation only) - factory_begin_run : BeginRun() callback (on run number change) - factory_change_run : ChangeRun() callback (on run number change) - factory_end_run : EndRun() callback (on run number change) The redundant 'phase' annotation is removed since the category name is now self-describing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/libraries/JANA/JFactory.cc | 18 +++++++++--------- src/libraries/JANA/Services/JPerfettoService.h | 5 ++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index f9ab5451b..cf7802b38 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -188,23 +188,23 @@ void JFactory::Create(const JEvent& event) { if (mPreviousRunNumber != -1) { { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "EndRun"); + TRACE_EVENT("factory_end_run", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number); #endif CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); } } { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); + TRACE_EVENT("factory_change_run", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number); #endif CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); } { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "BeginRun"); + TRACE_EVENT("factory_begin_run", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number); #endif CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); } @@ -212,8 +212,8 @@ void JFactory::Create(const JEvent& event) { else if (m_callback_style == CallbackStyle::ExpertMode) { { #if JANA2_HAVE_PERFETTO - TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number, "phase", "ChangeRun"); + TRACE_EVENT("factory_change_run", perfetto::DynamicString{GetFactoryName()}, + "tag", GetTag(), "run_nr", run_number); #endif CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); } @@ -281,7 +281,7 @@ void JFactory::DoInit() { { #if JANA2_HAVE_PERFETTO TRACE_EVENT("factory_init", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "phase", "Init"); + "tag", GetTag()); #endif CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); } diff --git a/src/libraries/JANA/Services/JPerfettoService.h b/src/libraries/JANA/Services/JPerfettoService.h index f6823b9ca..47cf13976 100644 --- a/src/libraries/JANA/Services/JPerfettoService.h +++ b/src/libraries/JANA/Services/JPerfettoService.h @@ -13,7 +13,10 @@ PERFETTO_DEFINE_CATEGORIES( perfetto::Category("jana").SetDescription("JANA2 framework events (arrow dispatch)"), perfetto::Category("factory").SetDescription("JANA2 factory Process() execution per event"), - perfetto::Category("factory_init").SetDescription("JANA2 factory Init/ChangeRun/BeginRun/EndRun callbacks") + perfetto::Category("factory_init").SetDescription("JANA2 factory Init() callback (first activation only)"), + perfetto::Category("factory_begin_run").SetDescription("JANA2 factory BeginRun() callback"), + perfetto::Category("factory_change_run").SetDescription("JANA2 factory ChangeRun() callback"), + perfetto::Category("factory_end_run").SetDescription("JANA2 factory EndRun() callback") ); #endif // JANA2_HAVE_PERFETTO From fbfec5b6a11a4c952920371a35c08ae2d6b83cf4 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 21 May 2026 10:35:16 -0500 Subject: [PATCH 7/9] perf: guard flow_id hash behind TRACE_EVENT_CATEGORY_ENABLED The FNV-1a hash (3 XOR+MUL ops) and GetEventNumber() ran unconditionally whenever a parent factory was present, even when the 'factory' category is disabled (no active tracing session). These are not protected by the lazy lambda evaluation inside TRACE_EVENT_INSTANT/BEGIN. Add an explicit TRACE_EVENT_CATEGORY_ENABLED("factory") check so the hash and all flow-related work is skipped entirely when tracing is off. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/libraries/JANA/JFactory.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index cf7802b38..11e19afc6 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -128,7 +128,10 @@ void JFactory::Create(const JEvent& event) { // Emit a flow-start instant inside the parent's still-open span, then open // our span with a TerminatingFlow — Perfetto draws an arrow parent → us. JFactory* const caller = g_current_executing_factory; - if (caller != nullptr) { + // Guard flow_id computation behind the category check: the FNV hash and + // GetEventNumber() are non-trivial work that should not run when tracing + // is disabled, even though the TRACE_EVENT_* args are already lazy. + if (TRACE_EVENT_CATEGORY_ENABLED("factory") && caller != nullptr) { // FNV-1a-inspired hash: unique 64-bit flow ID per (caller, callee, event). uint64_t flow_id = 14695981039346656037ULL; flow_id = (flow_id ^ reinterpret_cast(caller)) * 1099511628211ULL; From 9a25d643b08d10c1997efc0a7aeaa4c35a2092f1 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 21 May 2026 11:35:32 -0500 Subject: [PATCH 8/9] fix: address reviewer feedback on Perfetto integration Five issues raised in PR #497 review: 1. Add SHA256 integrity hash for Perfetto SDK download (supply-chain pinning). CMake will fail if the downloaded zip doesn't match. 2. Guard -Wno-* compile flags on perfetto_sdk with a CXX_COMPILER_ID generator expression (GNU/Clang/AppleClang only) so MSVC and other toolchains don't receive unknown flags. 3. factory_end_run span now records mPreviousRunNumber (the run being ended) instead of run_number (the new run). The previous value is the semantically correct label for the EndRun callback. 4. Flow-ID hash casts JFactory* through uintptr_t before widening to uint64_t, avoiding direct reinterpret_cast on a pointer which is non-portable on non-64-bit platforms. 5. JPerfettoService.cc is now always compiled (not gated on USE_PERFETTO). The file already contains a JANA2_HAVE_PERFETTO==0 stub, so downstream code that includes JPerfettoService.h will link correctly regardless of whether Perfetto was built in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CMakeLists.txt | 6 +++++- src/libraries/JANA/CMakeLists.txt | 6 +++--- src/libraries/JANA/JFactory.cc | 6 +++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e5d4dfe2e..6ddad7ca0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -146,6 +146,7 @@ if (USE_PERFETTO) file(DOWNLOAD "${PERFETTO_SDK_URL}" "${PERFETTO_SDK_ZIP}" STATUS DOWNLOAD_STATUS TLS_VERIFY ON + EXPECTED_HASH SHA256=443b148764c4259c9bf3e724bb508b66f3ff77d30e2ccfafbc0f1791cc08141c ) list(GET DOWNLOAD_STATUS 0 DOWNLOAD_ERROR_CODE) if(DOWNLOAD_ERROR_CODE) @@ -160,7 +161,10 @@ if (USE_PERFETTO) $ $ ) - target_compile_options(perfetto_sdk PRIVATE -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers) + target_compile_options(perfetto_sdk PRIVATE + $<$,$,$>: + -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers> + ) install(TARGETS perfetto_sdk EXPORT jana2_targets DESTINATION lib) install(FILES ${PERFETTO_SDK_DIR}/perfetto.h DESTINATION include/perfetto) set(JANA2_HAVE_PERFETTO 1) diff --git a/src/libraries/JANA/CMakeLists.txt b/src/libraries/JANA/CMakeLists.txt index 7ecc67237..e2e5a9fec 100644 --- a/src/libraries/JANA/CMakeLists.txt +++ b/src/libraries/JANA/CMakeLists.txt @@ -66,9 +66,9 @@ if (NOT ${USE_XERCES}) message(STATUS "Skipping support for libJANA's JGeometryXML because USE_XERCES=Off") endif() -if (USE_PERFETTO) - list(APPEND JANA2_SOURCES Services/JPerfettoService.cc) -endif() +# Always compiled — JPerfettoService.cc contains a JANA2_HAVE_PERFETTO==0 stub +# so it links correctly even when Perfetto support is not built in. +list(APPEND JANA2_SOURCES Services/JPerfettoService.cc) add_library(jana2 OBJECT ${JANA2_SOURCES}) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 11e19afc6..372ca9f2b 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -134,8 +134,8 @@ void JFactory::Create(const JEvent& event) { if (TRACE_EVENT_CATEGORY_ENABLED("factory") && caller != nullptr) { // FNV-1a-inspired hash: unique 64-bit flow ID per (caller, callee, event). uint64_t flow_id = 14695981039346656037ULL; - flow_id = (flow_id ^ reinterpret_cast(caller)) * 1099511628211ULL; - flow_id = (flow_id ^ reinterpret_cast(this)) * 1099511628211ULL; + flow_id = (flow_id ^ static_cast(reinterpret_cast(caller))) * 1099511628211ULL; + flow_id = (flow_id ^ static_cast(reinterpret_cast(this))) * 1099511628211ULL; flow_id = (flow_id ^ static_cast(event.GetEventNumber())) * 1099511628211ULL; // Flow starts here — we are still executing inside the parent's open span. TRACE_EVENT_INSTANT("factory", perfetto::DynamicString{GetFactoryName()}, @@ -192,7 +192,7 @@ void JFactory::Create(const JEvent& event) { { #if JANA2_HAVE_PERFETTO TRACE_EVENT("factory_end_run", perfetto::DynamicString{GetFactoryName()}, - "tag", GetTag(), "run_nr", run_number); + "tag", GetTag(), "run_nr", mPreviousRunNumber); #endif CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); } From 231a641f556a6414412c031d5d874b0ffecc7d1a Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 21 May 2026 11:45:29 -0500 Subject: [PATCH 9/9] fix: mv PERFETTO_HASH into separate cache variable --- CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ddad7ca0..6c6b9db74 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -137,6 +137,7 @@ endif() if (USE_PERFETTO) set(PERFETTO_VERSION "55.3" CACHE STRING "Perfetto SDK version to download") + set(PERFETTO_HASH "443b148764c4259c9bf3e724bb508b66f3ff77d30e2ccfafbc0f1791cc08141c" CACHE STRING "Perfetto zip sha256 hash") set(PERFETTO_SDK_URL "https://github.com/google/perfetto/releases/download/v${PERFETTO_VERSION}/perfetto-cpp-sdk-src.zip") set(PERFETTO_SDK_ZIP "${CMAKE_BINARY_DIR}/perfetto-cpp-sdk-src-${PERFETTO_VERSION}.zip") set(PERFETTO_SDK_DIR "${CMAKE_BINARY_DIR}/perfetto_sdk_${PERFETTO_VERSION}") @@ -146,7 +147,7 @@ if (USE_PERFETTO) file(DOWNLOAD "${PERFETTO_SDK_URL}" "${PERFETTO_SDK_ZIP}" STATUS DOWNLOAD_STATUS TLS_VERIFY ON - EXPECTED_HASH SHA256=443b148764c4259c9bf3e724bb508b66f3ff77d30e2ccfafbc0f1791cc08141c + EXPECTED_HASH SHA256=${PERFETTO_HASH} ) list(GET DOWNLOAD_STATUS 0 DOWNLOAD_ERROR_CODE) if(DOWNLOAD_ERROR_CODE)