From 37e3b0226958974defd14dd6d808e8557dcd7345 Mon Sep 17 00:00:00 2001 From: Jacob Hilliard <64656371+jcbhl@users.noreply.github.com> Date: Mon, 12 Jul 2021 09:57:45 -0700 Subject: [PATCH] Profiler: listen to OnSystemTick instead of OnFrameBegin (#1977) * Profiler: move to OnSystemTick polling Migrates from implementing FrameEventBus::Handler to SystemTickBus::Handler. When we were collecting data from threads on the start of a frame, it turns out that the event only fired once the RHI began working, which would exclude any events before that (ex. RPI, some AuxGeom regions) from the next GetTimeRegionMap call. Signed-off-by: Jacob Hilliard * Visualizer: update logic to avoid dangling frame Signed-off-by: Jacob Hilliard --- .../RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h | 9 ++++++--- Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp | 8 +++----- .../Code/Include/Atom/Utils/ImGuiCpuProfiler.inl | 14 +++++++++++--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h index eb35a29e26..697e58cdd4 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h @@ -9,13 +9,13 @@ #include #include +#include #include #include #include #include #include -#include namespace AZ { @@ -83,7 +83,7 @@ namespace AZ //! cached regions, which are stored on a per thread frequency. class CpuProfilerImpl final : public CpuProfiler - , public FrameEventBus::Handler + , public SystemTickBus::Handler { friend class CpuTimingLocalStorage; @@ -99,7 +99,10 @@ namespace AZ //! Unregisters the CpuProfilerImpl instance from the interface void Shutdown(); - void OnFrameBegin(); + // SystemTickBus::Handler overrides + // When fired, the profiler collects all profiling data from registered threads and updates + // m_timeRegionMap so that the next frame has up-to-date profiling data. + void OnSystemTick() final override; //! CpuProfiler overrides... void BeginTimeRegion(TimeRegion& timeRegion) final; diff --git a/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp b/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp index b31fbca6f2..187dfca1d9 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp @@ -79,8 +79,7 @@ namespace AZ { Interface::Register(this); m_initialized = true; - Device* rhiDevice = GetRHIDevice().get(); - FrameEventBus::Handler::BusConnect(rhiDevice); + SystemTickBus::Handler::BusConnect(); } void CpuProfilerImpl::Shutdown() @@ -101,7 +100,7 @@ namespace AZ m_registeredThreads.clear(); m_timeRegionMap.clear(); m_initialized = false; - FrameEventBus::Handler::BusDisconnect(); + SystemTickBus::Handler::BusDisconnect(); } void CpuProfilerImpl::BeginTimeRegion(TimeRegion& timeRegion) @@ -173,7 +172,7 @@ namespace AZ return m_enabled; } - void CpuProfilerImpl::OnFrameBegin() + void CpuProfilerImpl::OnSystemTick() { if (!m_enabled) { @@ -199,7 +198,6 @@ namespace AZ m_timeRegionMap = AZStd::move(newMap); } - void CpuProfilerImpl::RegisterThreadStorage() { AZStd::unique_lock lock(m_threadRegisterMutex); diff --git a/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl b/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl index e349685fd3..cc1bc9b3c0 100644 --- a/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl +++ b/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl @@ -649,7 +649,10 @@ namespace AZ // End ticks are sorted in increasing order, find the first frame bound to draw auto endTickItr = AZStd::lower_bound(m_frameEndTicks.begin(), m_frameEndTicks.end(), m_viewportStartTick); - while (endTickItr != m_frameEndTicks.end() && *endTickItr < m_viewportEndTick) + // Draw to one element before the last collected boundary if possible to avoid empty frame at the end + auto drawToItr = m_frameEndTicks.size() > 1 ? m_frameEndTicks.end() - 1 : m_frameEndTicks.end(); + + while (endTickItr != drawToItr && *endTickItr < m_viewportEndTick) { const float horizontalPixel = ConvertTickToPixelSpace(*endTickItr); drawList->AddLine({ horizontalPixel, wy }, { horizontalPixel, wy + windowHeight }, red); @@ -670,7 +673,9 @@ namespace AZ const auto [wx, wy] = ImGui::GetWindowPos(); ImDrawList* drawList = ImGui::GetWindowDrawList(); - while (nextFrameBoundaryItr != m_frameEndTicks.end()) + auto drawToItr = m_frameEndTicks.size() > 1 ? m_frameEndTicks.end() - 1 : m_frameEndTicks.end(); + + while (nextFrameBoundaryItr != drawToItr && *lastFrameBoundaryItr <= m_viewportEndTick) { const AZStd::sys_time_t lastFrameBoundaryTick = *lastFrameBoundaryItr; const AZStd::sys_time_t nextFrameBoundaryTick = *nextFrameBoundaryItr; @@ -750,7 +755,10 @@ namespace AZ // System tick bus overrides inline void ImGuiCpuProfiler::OnSystemTick() { - m_frameEndTicks.push_back(AZStd::GetTimeNowTicks()); + if (!m_paused) + { + m_frameEndTicks.push_back(AZStd::GetTimeNowTicks()); + } if (!m_showVisualizer || m_paused) {