Skip to content

Commit

Permalink
[Impeller] Give Impeller a dedicated raster priority level worker loo…
Browse files Browse the repository at this point in the history
…p. (flutter#43166)

We'd like to (or already are) using the concurrent message loop for high priority rendering tasks like PSO construction and render pass encoding. The default priority level for the engine managed concurrent message loop is 2, which is a significantly lower priority than the raster thread at -5. This is almost certainly causing priority inversion.

We must move back to dedicated runners so we can adjust thread priorities.
  • Loading branch information
jonahwilliams authored Jun 27, 2023
1 parent 77a27b4 commit de489e3
Show file tree
Hide file tree
Showing 39 changed files with 156 additions and 185 deletions.
1 change: 0 additions & 1 deletion ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@
../../../flutter/shell/platform/android/.gitignore
../../../flutter/shell/platform/android/android_context_gl_impeller_unittests.cc
../../../flutter/shell/platform/android/android_context_gl_unittests.cc
../../../flutter/shell/platform/android/android_context_vulkan_impeller_unittests.cc
../../../flutter/shell/platform/android/android_shell_holder_unittests.cc
../../../flutter/shell/platform/android/apk_asset_provider_unittests.cc
../../../flutter/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc
Expand Down
1 change: 1 addition & 0 deletions fml/concurrent_message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ ConcurrentMessageLoop::ConcurrentMessageLoop(size_t worker_count)
ConcurrentMessageLoop::~ConcurrentMessageLoop() {
Terminate();
for (auto& worker : workers_) {
FML_DCHECK(worker.joinable());
worker.join();
}
}
Expand Down
7 changes: 3 additions & 4 deletions impeller/playground/backend/metal/playground_impl_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,9 @@
if (!window) {
return;
}
auto worker_task_runner = concurrent_loop_->GetTaskRunner();
auto context = ContextMTL::Create(
ShaderLibraryMappingsForPlayground(), worker_task_runner,
is_gpu_disabled_sync_switch_, "Playground Library");
auto context =
ContextMTL::Create(ShaderLibraryMappingsForPlayground(),
is_gpu_disabled_sync_switch_, "Playground Library");
if (!context) {
return;
}
Expand Down
5 changes: 1 addition & 4 deletions impeller/playground/backend/vulkan/playground_impl_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ void PlaygroundImplVK::DestroyWindowHandle(WindowHandle handle) {
}

PlaygroundImplVK::PlaygroundImplVK(PlaygroundSwitches switches)
: PlaygroundImpl(switches),
concurrent_loop_(fml::ConcurrentMessageLoop::Create()),
handle_(nullptr, &DestroyWindowHandle) {
: PlaygroundImpl(switches), handle_(nullptr, &DestroyWindowHandle) {
if (!::glfwVulkanSupported()) {
#ifdef TARGET_OS_MAC
VALIDATION_LOG << "Attempted to initialize a Vulkan playground on macOS "
Expand Down Expand Up @@ -86,7 +84,6 @@ PlaygroundImplVK::PlaygroundImplVK(PlaygroundSwitches switches)
&::glfwGetInstanceProcAddress);
context_settings.shader_libraries_data = ShaderLibraryMappingsForPlayground();
context_settings.cache_directory = fml::paths::GetCachesDirectory();
context_settings.worker_task_runner = concurrent_loop_->GetTaskRunner();
context_settings.enable_validation = switches_.enable_vulkan_validation;

auto context = ContextVK::Create(std::move(context_settings));
Expand Down
2 changes: 0 additions & 2 deletions impeller/playground/backend/vulkan/playground_impl_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#pragma once

#include "flutter/fml/concurrent_message_loop.h"
#include "flutter/fml/macros.h"
#include "impeller/playground/playground_impl.h"
#include "impeller/renderer/backend/vulkan/vk.h"
Expand All @@ -18,7 +17,6 @@ class PlaygroundImplVK final : public PlaygroundImpl {
~PlaygroundImplVK();

private:
std::shared_ptr<fml::ConcurrentMessageLoop> concurrent_loop_;
std::shared_ptr<Context> context_;

// Windows management.
Expand Down
3 changes: 3 additions & 0 deletions impeller/playground/playground.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ void Playground::SetupWindow() {
}

void Playground::TeardownWindow() {
if (context_) {
context_->Shutdown();
}
context_.reset();
renderer_.reset();
impl_.reset();
Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/gles/context_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ bool ContextGLES::IsValid() const {
return is_valid_;
}

void ContextGLES::Shutdown() {}

// |Context|
std::string ContextGLES::DescribeGpuModel() const {
return reactor_->GetProcTable().GetDescription()->GetString();
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/context_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class ContextGLES final : public Context,
// |Context|
const std::shared_ptr<const Capabilities>& GetCapabilities() const override;

// |Context|
void Shutdown() override;

FML_DISALLOW_COPY_AND_ASSIGN(ContextGLES);
};

Expand Down
57 changes: 32 additions & 25 deletions impeller/renderer/backend/metal/command_buffer_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -202,32 +202,39 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
return false;
}

auto task = fml::MakeCopyable([render_pass, buffer, render_command_encoder,
weak_context = context_]() {
auto context = weak_context.lock();
if (!context) {
return;
}
auto is_gpu_disabled_sync_switch =
ContextMTL::Cast(*context).GetIsGpuDisabledSyncSwitch();
is_gpu_disabled_sync_switch->Execute(fml::SyncSwitch::Handlers().SetIfFalse(
[&render_pass, &render_command_encoder, &buffer, &context] {
auto mtl_render_pass = static_cast<RenderPassMTL*>(render_pass.get());
if (!mtl_render_pass->label_.empty()) {
[render_command_encoder
setLabel:@(mtl_render_pass->label_.c_str())];
}

auto result = mtl_render_pass->EncodeCommands(
context->GetResourceAllocator(), render_command_encoder);
auto task = fml::MakeCopyable(
[render_pass, buffer, render_command_encoder, weak_context = context_]() {
auto context = weak_context.lock();
if (!context) {
[render_command_encoder endEncoding];
if (result) {
[buffer commit];
} else {
VALIDATION_LOG << "Failed to encode command buffer";
}
}));
});
return;
}
auto is_gpu_disabled_sync_switch =
ContextMTL::Cast(*context).GetIsGpuDisabledSyncSwitch();
is_gpu_disabled_sync_switch->Execute(
fml::SyncSwitch::Handlers()
.SetIfFalse([&render_pass, &render_command_encoder, &buffer,
&context] {
auto mtl_render_pass =
static_cast<RenderPassMTL*>(render_pass.get());
if (!mtl_render_pass->label_.empty()) {
[render_command_encoder
setLabel:@(mtl_render_pass->label_.c_str())];
}

auto result = mtl_render_pass->EncodeCommands(
context->GetResourceAllocator(), render_command_encoder);
[render_command_encoder endEncoding];
if (result) {
[buffer commit];
} else {
VALIDATION_LOG << "Failed to encode command buffer";
}
})
.SetIfTrue([&render_command_encoder] {
[render_command_encoder endEncoding];
}));
});
worker_task_runner->PostTask(task);
return true;
}
Expand Down
11 changes: 5 additions & 6 deletions impeller/renderer/backend/metal/context_mtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,17 @@ class ContextMTL final : public Context,
public:
static std::shared_ptr<ContextMTL> Create(
const std::vector<std::string>& shader_library_paths,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch);

static std::shared_ptr<ContextMTL> Create(
const std::vector<std::shared_ptr<fml::Mapping>>& shader_libraries_data,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch,
const std::string& label);

static std::shared_ptr<ContextMTL> Create(
id<MTLDevice> device,
id<MTLCommandQueue> command_queue,
const std::vector<std::shared_ptr<fml::Mapping>>& shader_libraries_data,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch,
const std::string& label);

Expand Down Expand Up @@ -84,9 +81,12 @@ class ContextMTL final : public Context,
// |Context|
bool UpdateOffscreenLayerPixelFormat(PixelFormat format) override;

// |Context|
void Shutdown() override;

id<MTLCommandBuffer> CreateMTLCommandBuffer(const std::string& label) const;

const std::shared_ptr<fml::ConcurrentTaskRunner>& GetWorkerTaskRunner() const;
const std::shared_ptr<fml::ConcurrentTaskRunner> GetWorkerTaskRunner() const;

std::shared_ptr<const fml::SyncSwitch> GetIsGpuDisabledSyncSwitch() const;

Expand All @@ -98,15 +98,14 @@ class ContextMTL final : public Context,
std::shared_ptr<SamplerLibrary> sampler_library_;
std::shared_ptr<AllocatorMTL> resource_allocator_;
std::shared_ptr<const Capabilities> device_capabilities_;
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner_;
std::shared_ptr<fml::ConcurrentMessageLoop> raster_message_loop_;
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch_;
bool is_valid_ = false;

ContextMTL(
id<MTLDevice> device,
id<MTLCommandQueue> command_queue,
NSArray<id<MTLLibrary>>* shader_libraries,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch);

std::shared_ptr<CommandBuffer> CreateCommandBufferInQueue(
Expand Down
54 changes: 36 additions & 18 deletions impeller/renderer/backend/metal/context_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,34 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
id<MTLDevice> device,
id<MTLCommandQueue> command_queue,
NSArray<id<MTLLibrary>>* shader_libraries,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch)
: device_(device),
command_queue_(command_queue),
worker_task_runner_(std::move(worker_task_runner)),
is_gpu_disabled_sync_switch_(std::move(is_gpu_disabled_sync_switch)) {
// Validate device.
if (!device_) {
VALIDATION_LOG << "Could not setup valid Metal device.";
return;
}

// Worker task runner.
{
raster_message_loop_ = fml::ConcurrentMessageLoop::Create(
std::min(4u, std::thread::hardware_concurrency()));
raster_message_loop_->PostTaskToAllWorkers([]() {
// See https://github.com/flutter/flutter/issues/65752
// Intentionally opt out of QoS for raster task workloads.
[[NSThread currentThread] setThreadPriority:1.0];
sched_param param;
int policy;
pthread_t thread = pthread_self();
if (!pthread_getschedparam(thread, &policy, &param)) {
param.sched_priority = 50;
pthread_setschedparam(thread, policy, &param);
}
});
}

// Setup the shader library.
{
if (shader_libraries == nil) {
Expand Down Expand Up @@ -210,7 +226,6 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {

std::shared_ptr<ContextMTL> ContextMTL::Create(
const std::vector<std::string>& shader_library_paths,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch) {
auto device = CreateMetalDevice();
auto command_queue = CreateMetalCommandQueue(device);
Expand All @@ -220,7 +235,7 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
auto context = std::shared_ptr<ContextMTL>(new ContextMTL(
device, command_queue,
MTLShaderLibraryFromFilePaths(device, shader_library_paths),
std::move(worker_task_runner), std::move(is_gpu_disabled_sync_switch)));
std::move(is_gpu_disabled_sync_switch)));
if (!context->IsValid()) {
FML_LOG(ERROR) << "Could not create Metal context.";
return nullptr;
Expand All @@ -230,19 +245,18 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {

std::shared_ptr<ContextMTL> ContextMTL::Create(
const std::vector<std::shared_ptr<fml::Mapping>>& shader_libraries_data,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch,
const std::string& library_label) {
auto device = CreateMetalDevice();
auto command_queue = CreateMetalCommandQueue(device);
if (!command_queue) {
return nullptr;
}
auto context = std::shared_ptr<ContextMTL>(new ContextMTL(
device, command_queue,
MTLShaderLibraryFromFileData(device, shader_libraries_data,
library_label),
std::move(worker_task_runner), std::move(is_gpu_disabled_sync_switch)));
auto context = std::shared_ptr<ContextMTL>(
new ContextMTL(device, command_queue,
MTLShaderLibraryFromFileData(device, shader_libraries_data,
library_label),
std::move(is_gpu_disabled_sync_switch)));
if (!context->IsValid()) {
FML_LOG(ERROR) << "Could not create Metal context.";
return nullptr;
Expand All @@ -254,14 +268,13 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
id<MTLDevice> device,
id<MTLCommandQueue> command_queue,
const std::vector<std::shared_ptr<fml::Mapping>>& shader_libraries_data,
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner,
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch,
const std::string& library_label) {
auto context = std::shared_ptr<ContextMTL>(new ContextMTL(
device, command_queue,
MTLShaderLibraryFromFileData(device, shader_libraries_data,
library_label),
std::move(worker_task_runner), std::move(is_gpu_disabled_sync_switch)));
auto context = std::shared_ptr<ContextMTL>(
new ContextMTL(device, command_queue,
MTLShaderLibraryFromFileData(device, shader_libraries_data,
library_label),
std::move(is_gpu_disabled_sync_switch)));
if (!context->IsValid()) {
FML_LOG(ERROR) << "Could not create Metal context.";
return nullptr;
Expand Down Expand Up @@ -301,9 +314,14 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
return CreateCommandBufferInQueue(command_queue_);
}

const std::shared_ptr<fml::ConcurrentTaskRunner>&
// |Context|
void ContextMTL::Shutdown() {
raster_message_loop_.reset();
}

const std::shared_ptr<fml::ConcurrentTaskRunner>
ContextMTL::GetWorkerTaskRunner() const {
return worker_task_runner_;
return raster_message_loop_->GetTaskRunner();
}

std::shared_ptr<const fml::SyncSwitch> ContextMTL::GetIsGpuDisabledSyncSwitch()
Expand Down
35 changes: 24 additions & 11 deletions impeller/renderer/backend/vulkan/context_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

#include "impeller/renderer/backend/vulkan/context_vk.h"

#ifdef FML_OS_ANDROID
#include <pthread.h>
#include <sys/resource.h>
#include <sys/time.h>
#endif // FML_OS_ANDROID

#include <map>
#include <memory>
#include <optional>
Expand Down Expand Up @@ -120,12 +126,15 @@ void ContextVK::Setup(Settings settings) {
return;
}

if (!settings.worker_task_runner) {
VALIDATION_LOG
<< "Cannot set up a Vulkan context without a worker task runner.";
return;
}
worker_task_runner_ = settings.worker_task_runner;
raster_message_loop_ = fml::ConcurrentMessageLoop::Create(
std::min(4u, std::thread::hardware_concurrency()));
#ifdef FML_OS_ANDROID
raster_message_loop_->PostTaskToAllWorkers([]() {
if (::setpriority(PRIO_PROCESS, gettid(), -5) != 0) {
FML_LOG(ERROR) << "Failed to set Workers task runner priority";
}
});
#endif // FML_OS_ANDROID

auto& dispatcher = VULKAN_HPP_DEFAULT_DISPATCHER;
dispatcher.init(settings.proc_address_callback);
Expand Down Expand Up @@ -335,10 +344,10 @@ void ContextVK::Setup(Settings settings) {
/// Setup the pipeline library.
///
auto pipeline_library = std::shared_ptr<PipelineLibraryVK>(
new PipelineLibraryVK(device_holder, //
caps, //
std::move(settings.cache_directory), //
worker_task_runner_ //
new PipelineLibraryVK(device_holder, //
caps, //
std::move(settings.cache_directory), //
raster_message_loop_->GetTaskRunner() //
));

if (!pipeline_library->IsValid()) {
Expand Down Expand Up @@ -458,7 +467,11 @@ const vk::Device& ContextVK::GetDevice() const {

const std::shared_ptr<fml::ConcurrentTaskRunner>
ContextVK::GetConcurrentWorkerTaskRunner() const {
return worker_task_runner_;
return raster_message_loop_->GetTaskRunner();
}

void ContextVK::Shutdown() {
raster_message_loop_->Terminate();
}

std::unique_ptr<Surface> ContextVK::AcquireNextSurface() {
Expand Down
Loading

0 comments on commit de489e3

Please sign in to comment.