From 4df93fc1947437cfc0e0d1e203e036e3f6e1d277 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 13:39:54 -0700 Subject: [PATCH 01/11] [Impeller] add GPU start/end trace events. --- .../backend/vulkan/command_buffer_vk.cc | 3 ++ .../backend/vulkan/swapchain_impl_vk.cc | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 0059fe41b26f2..78eb547847082 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -52,6 +52,9 @@ const std::shared_ptr& CommandBufferVK::GetEncoder() { } bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) { + if (!encoder_) { + encoder_ = encoder_factory_->Create(); + } if (!callback) { return encoder_->Submit(); } diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 0af88001b8e95..946183a07baf7 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -4,16 +4,40 @@ #include "impeller/renderer/backend/vulkan/swapchain_impl_vk.h" +#include "fml/trace_event.h" +#include "impeller/base/validation.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/surface_vk.h" #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h" +#include "impeller/renderer/context.h" #include "vulkan/vulkan_structs.hpp" namespace impeller { +// Submit an empty cmd buffer and track its completion so that we can record +// the approximate time that the GPU workload started/ended in the Dart +// timeline. +static void RunGPUCompletionTracking(const ContextVK& context, bool start) { + auto cmd_buffer = context.CreateCommandBuffer(); + if (!cmd_buffer->SubmitCommands([start](CommandBuffer::Status status) { + if (status == CommandBuffer::Status::kPending) { + return; + } + if (start) { + TRACE_EVENT0("flutter", "GPUStart"); + } else { + TRACE_EVENT0("flutter", "GPUEnd"); + } + // We report this event regardless of the status to simplify the tracing + // code required to process the event. + })) { + VALIDATION_LOG << "Failed to submit tracking cmd buffer."; + } +} + static constexpr size_t kMaxFramesInFlight = 3u; // Number of frames to poll for orientation changes. For example `1u` means @@ -374,6 +398,9 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { *sync->render_ready, // signal semaphore nullptr // fence ); +#ifdef IMPELLER_DEBUG + RunGPUCompletionTracking(context, /*start=*/true); +#endif // IMPELLER_DEBUG switch (acq_result) { case vk::Result::eSuccess: @@ -421,6 +448,10 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, const auto& context = ContextVK::Cast(*context_strong); const auto& sync = synchronizers_[current_frame_]; +#ifdef IMPELLER_DEBUG + RunGPUCompletionTracking(context, /*start=*/false); +#endif // IMPELLER_DEBUG + //---------------------------------------------------------------------------- /// Transition the image to color-attachment-optimal. /// From 995bb5ed7e5e54ab87fa95d0971f370080185269 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 14:09:15 -0700 Subject: [PATCH 02/11] ++ --- impeller/renderer/backend/vulkan/swapchain_impl_vk.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 946183a07baf7..a0786d7764483 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -17,6 +17,7 @@ namespace impeller { +#ifdef IMPELLER_DEBUG // Submit an empty cmd buffer and track its completion so that we can record // the approximate time that the GPU workload started/ended in the Dart // timeline. @@ -37,6 +38,7 @@ static void RunGPUCompletionTracking(const ContextVK& context, bool start) { VALIDATION_LOG << "Failed to submit tracking cmd buffer."; } } +#endif // IMPELLER_DEBUG static constexpr size_t kMaxFramesInFlight = 3u; From 4060e8b21226528981ca9b21704683298ee4b952 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 20:21:10 -0700 Subject: [PATCH 03/11] timeline events. --- impeller/renderer/backend/vulkan/BUILD.gn | 2 + .../renderer/backend/vulkan/context_vk.cc | 9 ++ impeller/renderer/backend/vulkan/context_vk.h | 7 ++ .../renderer/backend/vulkan/gpu_tracer_vk.cc | 107 ++++++++++++++++++ .../renderer/backend/vulkan/gpu_tracer_vk.h | 40 +++++++ impeller/renderer/backend/vulkan/queue_vk.h | 2 + .../backend/vulkan/swapchain_impl_vk.cc | 38 ++----- 7 files changed, 174 insertions(+), 31 deletions(-) create mode 100644 impeller/renderer/backend/vulkan/gpu_tracer_vk.cc create mode 100644 impeller/renderer/backend/vulkan/gpu_tracer_vk.h diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index 6b49311ed35a2..5fde56ee9d607 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -105,6 +105,8 @@ impeller_component("vulkan") { "vk.h", "vma.cc", "vma.h", + "gpu_tracer_vk.h", + "gpu_tracer_vk.cc" ] public_deps = [ diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 5a8b0e98c85b2..df814dcac7a97 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -26,6 +26,7 @@ #include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/debug_report_vk.h" #include "impeller/renderer/backend/vulkan/fence_waiter_vk.h" +#include "impeller/renderer/backend/vulkan/gpu_tracer_vk.h" #include "impeller/renderer/backend/vulkan/resource_manager_vk.h" #include "impeller/renderer/backend/vulkan/surface_context_vk.h" #include "impeller/renderer/capabilities.h" @@ -430,6 +431,10 @@ void ContextVK::Setup(Settings settings) { device_name_ = std::string(physical_device_properties.deviceName); is_valid_ = true; + // Create the GPU Tracer later because it depends on state from + // the ContextVK. + gpu_tracer_ = std::make_shared(shared_from_this()); + //---------------------------------------------------------------------------- /// Label all the relevant objects. This happens after setup so that the /// debug messengers have had a chance to be set up. @@ -532,4 +537,8 @@ ContextVK::CreateGraphicsCommandEncoderFactory() const { return std::make_unique(weak_from_this()); } +std::shared_ptr ContextVK::GetGPUTracer() const { + return gpu_tracer_; +} + } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 144de15b26913..8a4a9fc2b4b5f 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -32,6 +32,7 @@ class DebugReportVK; class FenceWaiterVK; class ResourceManagerVK; class SurfaceContextVK; +class GPUTracerVK; class ContextVK final : public Context, public BackendCast, @@ -144,6 +145,10 @@ class ContextVK final : public Context, std::shared_ptr GetCommandPoolRecycler() const; + std::shared_ptr GetGPUTracer() const; + + void RecordFrameEndTime() const; + private: struct DeviceHolderImpl : public DeviceHolder { // |DeviceHolder| @@ -171,6 +176,8 @@ class ContextVK final : public Context, std::shared_ptr command_pool_recycler_; std::string device_name_; std::shared_ptr raster_message_loop_; + std::shared_ptr gpu_tracer_; + bool sync_presentation_ = false; const uint64_t hash_; diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc new file mode 100644 index 0000000000000..7743cd2e7d313 --- /dev/null +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -0,0 +1,107 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/renderer/backend/vulkan/gpu_tracer_vk.h" + +#include +#include "fml/trace_event.h" +#include "impeller/base/validation.h" +#include "impeller/renderer/backend/vulkan/command_buffer_vk.h" +#include "impeller/renderer/backend/vulkan/command_encoder_vk.h" +#include "impeller/renderer/backend/vulkan/context_vk.h" +#include "impeller/renderer/command_buffer.h" + +namespace impeller { + +static constexpr uint32_t kPoolSize = 128u; + +GPUTracerVK::GPUTracerVK(const std::shared_ptr& context) + : context_(context) { + timestamp_period_ = + context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; + + vk::QueryPoolCreateInfo info; + info.queryCount = kPoolSize; + info.queryType = vk::QueryType::eTimestamp; + + auto [status, pool] = context_->GetDevice().createQueryPoolUnique(info); + if (status != vk::Result::eSuccess) { + VALIDATION_LOG << "Failed to create query pool."; + return; + } + query_pool_ = std::move(pool); + valid_ = true; +} + +void GPUTracerVK::RecordStartFrameTime() { + if (!valid_) { + return; + } + auto buffer = context_->CreateCommandBuffer(); + auto vk_trace_cmd_buffer = + CommandBufferVK::Cast(*buffer).GetEncoder()->GetCommandBuffer(); + // The two commands below are executed in order, such that writeTimeStamp is + // guaranteed to occur after resetQueryPool has finished. The validation + // layer seem particularly strict, and efforts to reset the entire pool + // were met with validation errors (though seemingly correct measurements). + // To work around this, the tracer only resets the query that will be + // used next. + vk_trace_cmd_buffer.resetQueryPool(query_pool_.get(), current_index_, 1); + vk_trace_cmd_buffer.writeTimestamp(vk::PipelineStageFlagBits::eTopOfPipe, + query_pool_.get(), current_index_); + + if (!buffer->SubmitCommands()) { + VALIDATION_LOG << "GPUTracerVK: Failed to record start time."; + } + + // The logic in RecordEndFrameTime requires us to have recorded a pair of + // tracing events. If this method failed for any reason we need to be sure we + // don't attempt to record and read back a second value, or we will get values + // that span multiple frames. + started_frame_ = true; +} + +void GPUTracerVK::RecordEndFrameTime() { + if (!valid_ || !started_frame_) { + return; + } + started_frame_ = false; + auto last_query = current_index_; + current_index_ += 1; + + auto buffer = context_->CreateCommandBuffer(); + auto vk_trace_cmd_buffer = + CommandBufferVK::Cast(*buffer).GetEncoder()->GetCommandBuffer(); + vk_trace_cmd_buffer.resetQueryPool(query_pool_.get(), current_index_, 1); + vk_trace_cmd_buffer.writeTimestamp(vk::PipelineStageFlagBits::eBottomOfPipe, + query_pool_.get(), current_index_); + + // On completion of the second time stamp recording, we read back this value + // and the previous value. The difference is approximately the frame time. + if (!buffer->SubmitCommands([&, last_query](CommandBuffer::Status status) { + uint64_t bits[2] = {0, 0}; + auto result = context_->GetDevice().getQueryPoolResults( + query_pool_.get(), last_query, 2, sizeof(bits), &bits, + sizeof(int64_t), vk::QueryResultFlagBits::e64); + + if (result == vk::Result::eSuccess) { + // This value should probably be available in some form besides a + // timeline event but that is a job for a future Jonah. + auto gpu_ms = (((bits[1] - bits[0]) * timestamp_period_) / 1000000); + FML_TRACE_COUNTER("flutter", "GPUTracer", + 1234, // Trace Counter ID + "FrameTimeMS", gpu_ms); + } + })) { + if (!buffer->SubmitCommands()) { + VALIDATION_LOG << "GPUTracerVK failed to record frame end time."; + } + } + + if (current_index_ == kPoolSize - 1) { + current_index_ = 0u; + } +} + +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.h b/impeller/renderer/backend/vulkan/gpu_tracer_vk.h new file mode 100644 index 0000000000000..80f9645bf90b2 --- /dev/null +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.h @@ -0,0 +1,40 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include +#include "impeller/renderer/backend/vulkan/context_vk.h" +#include "impeller/renderer/backend/vulkan/vk.h" + +namespace impeller { + +/// @brief A class that uses timestamp queries to record the approximate GPU +/// execution time. +class GPUTracerVK { + public: + explicit GPUTracerVK(const std::shared_ptr& context); + + ~GPUTracerVK() = default; + + /// @brief Record the approximate start time of the GPU workload for the + /// current frame. + void RecordStartFrameTime(); + + /// @brief Record the approximate end time of the GPU workload for the current + /// frame. + void RecordEndFrameTime(); + + private: + void ResetQueryPool(size_t pool); + + const std::shared_ptr context_; + vk::UniqueQueryPool query_pool_ = {}; + + size_t current_index_ = 0u; + // The number of nanoseconds for each timestamp unit. + float timestamp_period_ = 1; + bool started_frame_ = false; + bool valid_ = false; +}; + +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/queue_vk.h b/impeller/renderer/backend/vulkan/queue_vk.h index 6ba3b9116fbdb..cf4358519eee4 100644 --- a/impeller/renderer/backend/vulkan/queue_vk.h +++ b/impeller/renderer/backend/vulkan/queue_vk.h @@ -41,6 +41,8 @@ class QueueVK { void InsertDebugMarker(const char* label) const; + size_t GetTimestampBits() const; + private: mutable Mutex queue_mutex_; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index a0786d7764483..d61ff4a3cf83a 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -4,12 +4,12 @@ #include "impeller/renderer/backend/vulkan/swapchain_impl_vk.h" -#include "fml/trace_event.h" #include "impeller/base/validation.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" +#include "impeller/renderer/backend/vulkan/gpu_tracer_vk.h" #include "impeller/renderer/backend/vulkan/surface_vk.h" #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h" #include "impeller/renderer/context.h" @@ -17,29 +17,6 @@ namespace impeller { -#ifdef IMPELLER_DEBUG -// Submit an empty cmd buffer and track its completion so that we can record -// the approximate time that the GPU workload started/ended in the Dart -// timeline. -static void RunGPUCompletionTracking(const ContextVK& context, bool start) { - auto cmd_buffer = context.CreateCommandBuffer(); - if (!cmd_buffer->SubmitCommands([start](CommandBuffer::Status status) { - if (status == CommandBuffer::Status::kPending) { - return; - } - if (start) { - TRACE_EVENT0("flutter", "GPUStart"); - } else { - TRACE_EVENT0("flutter", "GPUEnd"); - } - // We report this event regardless of the status to simplify the tracing - // code required to process the event. - })) { - VALIDATION_LOG << "Failed to submit tracking cmd buffer."; - } -} -#endif // IMPELLER_DEBUG - static constexpr size_t kMaxFramesInFlight = 3u; // Number of frames to poll for orientation changes. For example `1u` means @@ -400,9 +377,9 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { *sync->render_ready, // signal semaphore nullptr // fence ); -#ifdef IMPELLER_DEBUG - RunGPUCompletionTracking(context, /*start=*/true); -#endif // IMPELLER_DEBUG + + /// Record the approximate start of the GPU workload. + context.GetGPUTracer()->RecordStartFrameTime(); switch (acq_result) { case vk::Result::eSuccess: @@ -450,10 +427,6 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, const auto& context = ContextVK::Cast(*context_strong); const auto& sync = synchronizers_[current_frame_]; -#ifdef IMPELLER_DEBUG - RunGPUCompletionTracking(context, /*start=*/false); -#endif // IMPELLER_DEBUG - //---------------------------------------------------------------------------- /// Transition the image to color-attachment-optimal. /// @@ -483,6 +456,9 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, } } + /// Record the approximate end of the GPU workload. + context.GetGPUTracer()->RecordEndFrameTime(); + //---------------------------------------------------------------------------- /// Signal that the presentation semaphore is ready. /// From dd6e71d6cec9e047f61934a0bac5c3fb11608d4e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 20:21:35 -0700 Subject: [PATCH 04/11] ++ --- impeller/renderer/backend/vulkan/BUILD.gn | 4 ++-- impeller/renderer/backend/vulkan/context_vk.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index 5fde56ee9d607..0a7702f36a566 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -61,6 +61,8 @@ impeller_component("vulkan") { "fence_waiter_vk.h", "formats_vk.cc", "formats_vk.h", + "gpu_tracer_vk.cc", + "gpu_tracer_vk.h", "limits_vk.h", "pass_bindings_cache.cc", "pass_bindings_cache.h", @@ -105,8 +107,6 @@ impeller_component("vulkan") { "vk.h", "vma.cc", "vma.h", - "gpu_tracer_vk.h", - "gpu_tracer_vk.cc" ] public_deps = [ diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 8a4a9fc2b4b5f..0febc8b03da15 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -145,7 +145,7 @@ class ContextVK final : public Context, std::shared_ptr GetCommandPoolRecycler() const; - std::shared_ptr GetGPUTracer() const; + std::shared_ptr GetGPUTracer() const; void RecordFrameEndTime() const; From 096c7e65a30d8e1e1f0a9688e8694aaf51725719 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 20:50:04 -0700 Subject: [PATCH 05/11] ++ --- impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 2 +- impeller/renderer/backend/vulkan/queue_vk.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 7743cd2e7d313..0f5b15ac3cd9c 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -20,7 +20,7 @@ GPUTracerVK::GPUTracerVK(const std::shared_ptr& context) : context_(context) { timestamp_period_ = context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; - + FML_LOG(ERROR) << context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; vk::QueryPoolCreateInfo info; info.queryCount = kPoolSize; info.queryType = vk::QueryType::eTimestamp; diff --git a/impeller/renderer/backend/vulkan/queue_vk.h b/impeller/renderer/backend/vulkan/queue_vk.h index cf4358519eee4..6ba3b9116fbdb 100644 --- a/impeller/renderer/backend/vulkan/queue_vk.h +++ b/impeller/renderer/backend/vulkan/queue_vk.h @@ -41,8 +41,6 @@ class QueueVK { void InsertDebugMarker(const char* label) const; - size_t GetTimestampBits() const; - private: mutable Mutex queue_mutex_; From 915652f5839e3bd5c70697f7ba0406b206b29c0c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 20:51:37 -0700 Subject: [PATCH 06/11] ++ --- ci/licenses_golden/licenses_flutter | 4 ++++ impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index b8e32aa31564f..bf957a0389f8b 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2184,6 +2184,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc + . ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc + ../../../flutter/LICENSE @@ -4945,6 +4947,8 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h +FILE: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +FILE: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/limits_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.h diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 0f5b15ac3cd9c..d53d35db2d383 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -20,7 +20,8 @@ GPUTracerVK::GPUTracerVK(const std::shared_ptr& context) : context_(context) { timestamp_period_ = context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; - FML_LOG(ERROR) << context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; + FML_LOG(ERROR) + << context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; vk::QueryPoolCreateInfo info; info.queryCount = kPoolSize; info.queryType = vk::QueryType::eTimestamp; From 12870c3572ba62f75b5fe1c7de9299e1a294b72b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Oct 2023 21:18:54 -0700 Subject: [PATCH 07/11] dnfield review --- .../renderer/backend/vulkan/context_vk.cc | 2 +- .../renderer/backend/vulkan/gpu_tracer_vk.cc | 42 ++++++++++++++----- .../renderer/backend/vulkan/gpu_tracer_vk.h | 6 +-- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index df814dcac7a97..028b92ffb1be4 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -433,7 +433,7 @@ void ContextVK::Setup(Settings settings) { // Create the GPU Tracer later because it depends on state from // the ContextVK. - gpu_tracer_ = std::make_shared(shared_from_this()); + gpu_tracer_ = std::make_shared(weak_from_this()); //---------------------------------------------------------------------------- /// Label all the relevant objects. This happens after setup so that the diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index d53d35db2d383..db740421fe748 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -16,17 +16,24 @@ namespace impeller { static constexpr uint32_t kPoolSize = 128u; -GPUTracerVK::GPUTracerVK(const std::shared_ptr& context) +GPUTracerVK::GPUTracerVK(const std::weak_ptr& context) : context_(context) { - timestamp_period_ = - context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; - FML_LOG(ERROR) - << context_->GetPhysicalDevice().getProperties().limits.timestampPeriod; + auto strong_context = context_.lock(); + if (!strong_context) { + return; + } + timestamp_period_ = strong_context->GetPhysicalDevice() + .getProperties() + .limits.timestampPeriod; + if (timestamp_period_ <= 0) { + // The device does not support timestamp queries. + return; + } vk::QueryPoolCreateInfo info; info.queryCount = kPoolSize; info.queryType = vk::QueryType::eTimestamp; - auto [status, pool] = context_->GetDevice().createQueryPoolUnique(info); + auto [status, pool] = strong_context->GetDevice().createQueryPoolUnique(info); if (status != vk::Result::eSuccess) { VALIDATION_LOG << "Failed to create query pool."; return; @@ -39,7 +46,11 @@ void GPUTracerVK::RecordStartFrameTime() { if (!valid_) { return; } - auto buffer = context_->CreateCommandBuffer(); + auto strong_context = context_.lock(); + if (!strong_context) { + return; + } + auto buffer = strong_context->CreateCommandBuffer(); auto vk_trace_cmd_buffer = CommandBufferVK::Cast(*buffer).GetEncoder()->GetCommandBuffer(); // The two commands below are executed in order, such that writeTimeStamp is @@ -67,11 +78,16 @@ void GPUTracerVK::RecordEndFrameTime() { if (!valid_ || !started_frame_) { return; } + auto strong_context = context_.lock(); + if (!strong_context) { + return; + } + started_frame_ = false; auto last_query = current_index_; current_index_ += 1; - auto buffer = context_->CreateCommandBuffer(); + auto buffer = strong_context->CreateCommandBuffer(); auto vk_trace_cmd_buffer = CommandBufferVK::Cast(*buffer).GetEncoder()->GetCommandBuffer(); vk_trace_cmd_buffer.resetQueryPool(query_pool_.get(), current_index_, 1); @@ -80,9 +96,15 @@ void GPUTracerVK::RecordEndFrameTime() { // On completion of the second time stamp recording, we read back this value // and the previous value. The difference is approximately the frame time. - if (!buffer->SubmitCommands([&, last_query](CommandBuffer::Status status) { + const auto device_holder = strong_context->GetDeviceHolder(); + if (!buffer->SubmitCommands([&, last_query, + device_holder](CommandBuffer::Status status) { + auto strong_context = context_.lock(); + if (!strong_context) { + return; + } uint64_t bits[2] = {0, 0}; - auto result = context_->GetDevice().getQueryPoolResults( + auto result = device_holder->GetDevice().getQueryPoolResults( query_pool_.get(), last_query, 2, sizeof(bits), &bits, sizeof(int64_t), vk::QueryResultFlagBits::e64); diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.h b/impeller/renderer/backend/vulkan/gpu_tracer_vk.h index 80f9645bf90b2..8913bf3ffdb08 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.h +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.h @@ -3,8 +3,8 @@ // found in the LICENSE file. #include + #include "impeller/renderer/backend/vulkan/context_vk.h" -#include "impeller/renderer/backend/vulkan/vk.h" namespace impeller { @@ -12,7 +12,7 @@ namespace impeller { /// execution time. class GPUTracerVK { public: - explicit GPUTracerVK(const std::shared_ptr& context); + explicit GPUTracerVK(const std::weak_ptr& context); ~GPUTracerVK() = default; @@ -27,7 +27,7 @@ class GPUTracerVK { private: void ResetQueryPool(size_t pool); - const std::shared_ptr context_; + const std::weak_ptr context_; vk::UniqueQueryPool query_pool_ = {}; size_t current_index_ = 0u; From 4910e589d560280b27238a09fb742dff603defa1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 13 Oct 2023 12:31:25 -0700 Subject: [PATCH 08/11] ++ --- impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index db740421fe748..159c7f9501cc0 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -39,7 +39,10 @@ GPUTracerVK::GPUTracerVK(const std::weak_ptr& context) return; } query_pool_ = std::move(pool); + // Disable tracing in release mode. +#ifdef IMPELLER_DEBUG valid_ = true; +#endif } void GPUTracerVK::RecordStartFrameTime() { From e35c2efe39b8edb1917ef4e1be468562b8956152 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 13 Oct 2023 13:11:22 -0700 Subject: [PATCH 09/11] Update gpu_tracer_vk.cc --- impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 159c7f9501cc0..888e1bfde9cd3 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -116,7 +116,7 @@ void GPUTracerVK::RecordEndFrameTime() { // timeline event but that is a job for a future Jonah. auto gpu_ms = (((bits[1] - bits[0]) * timestamp_period_) / 1000000); FML_TRACE_COUNTER("flutter", "GPUTracer", - 1234, // Trace Counter ID + reinterpret_cast(this), // Trace Counter ID "FrameTimeMS", gpu_ms); } })) { From f02f6091bf18dfda3590ccb8268544150e44fc63 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 13 Oct 2023 14:07:41 -0700 Subject: [PATCH 10/11] ++ --- impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 888e1bfde9cd3..34fef07a80e90 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -63,7 +63,7 @@ void GPUTracerVK::RecordStartFrameTime() { // To work around this, the tracer only resets the query that will be // used next. vk_trace_cmd_buffer.resetQueryPool(query_pool_.get(), current_index_, 1); - vk_trace_cmd_buffer.writeTimestamp(vk::PipelineStageFlagBits::eTopOfPipe, + vk_trace_cmd_buffer.writeTimestamp(vk::PipelineStageFlagBits::eBottomOfPipe, query_pool_.get(), current_index_); if (!buffer->SubmitCommands()) { From 635ebb0e5d70e8bdfe052b4f3ca8396ba6efb268 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 13 Oct 2023 14:08:13 -0700 Subject: [PATCH 11/11] ++ --- impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 34fef07a80e90..41614ab708f54 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -115,9 +115,10 @@ void GPUTracerVK::RecordEndFrameTime() { // This value should probably be available in some form besides a // timeline event but that is a job for a future Jonah. auto gpu_ms = (((bits[1] - bits[0]) * timestamp_period_) / 1000000); - FML_TRACE_COUNTER("flutter", "GPUTracer", - reinterpret_cast(this), // Trace Counter ID - "FrameTimeMS", gpu_ms); + FML_TRACE_COUNTER( + "flutter", "GPUTracer", + reinterpret_cast(this), // Trace Counter ID + "FrameTimeMS", gpu_ms); } })) { if (!buffer->SubmitCommands()) {