Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Remove meaningless judgments for gpu task when raster and platform are not same thread #25583

Closed
wants to merge 5 commits into from
Closed

Remove meaningless judgments for gpu task when raster and platform are not same thread #25583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d3f34c8
remove judgment for gpu task
kinarobin Apr 14, 2021
30cc9dc
delete unused parameters
kinarobin Apr 14, 2021
bd95d32
remove meaningless test case
kinarobin Apr 15, 2021
9f9a250
remove meaningless test case
kinarobin Apr 15, 2021
498f4f9
update test case
kinarobin Apr 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shell/common/rasterizer_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ TEST(
fml::MessageLoop::EnsureInitializedForCurrentThread();
TaskRunners task_runners("test",
fml::MessageLoop::GetCurrent().GetTaskRunner(),
fml::MessageLoop::GetCurrent().GetTaskRunner(),
thread_host.raster_thread->GetTaskRunner(),
thread_host.ui_thread->GetTaskRunner(),
thread_host.io_thread->GetTaskRunner());

Expand Down
75 changes: 4 additions & 71 deletions shell/common/shell.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,28 +716,8 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {

// Prevent any request to change the thread configuration for raster and
// platform queues while the platform view is being created.
//
// This prevents false positives such as this method starts assuming that the
// raster and platform queues have a given thread configuration, but then the
// configuration is changed by a task, and the asumption is not longer true.
//
// This incorrect assumption can lead to dead lock.
// See `should_post_raster_task` for more.
rasterizer_->DisableThreadMergerIfNeeded();

// The normal flow executed by this method is that the platform thread is
// starting the sequence and waiting on the latch. Later the UI thread posts
// raster_task to the raster thread which signals the latch. If the raster and
// the platform threads are the same this results in a deadlock as the
// raster_task will never be posted to the plaform/raster thread that is
// blocked on a latch. To avoid the described deadlock, if the raster and the
// platform threads are the same, should_post_raster_task will be false, and
// then instead of posting a task to the raster thread, the ui thread just
// signals the latch and the platform/raster thread follows with executing
// raster_task.
const bool should_post_raster_task =
!task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread();

// Note:
// This is a synchronous operation because certain platforms depend on
// setup/suspension of all activities that may be interacting with the GPU in
Expand All @@ -764,21 +744,13 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {

auto ui_task = [engine = engine_->GetWeakPtr(), //
raster_task_runner = task_runners_.GetRasterTaskRunner(), //
raster_task, should_post_raster_task,
&latch //
] {
raster_task]() {
if (engine) {
engine->OnOutputSurfaceCreated();
}
// Step 2: Next, tell the raster thread that it should create a surface for
// its rasterizer.
if (should_post_raster_task) {
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
} else {
// See comment on should_post_raster_task, in this case we just unblock
// the platform thread.
latch.Signal();
}
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
};

// Threading: Capture platform view by raw pointer and not the weak pointer.
Expand Down Expand Up @@ -809,12 +781,6 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);

latch.Wait();
if (!should_post_raster_task) {
// See comment on should_post_raster_task, in this case the raster_task
// wasn't executed, and we just run it here as the platform thread
// is the raster thread.
raster_task();
}
}

// |PlatformView::Delegate|
Expand All @@ -825,28 +791,8 @@ void Shell::OnPlatformViewDestroyed() {

// Prevent any request to change the thread configuration for raster and
// platform queues while the platform view is being destroyed.
//
// This prevents false positives such as this method starts assuming that the
// raster and platform queues have a given thread configuration, but then the
// configuration is changed by a task, and the asumption is not longer true.
//
// This incorrect assumption can lead to dead lock.
// See `should_post_raster_task` for more.
rasterizer_->DisableThreadMergerIfNeeded();

// The normal flow executed by this method is that the platform thread is
// starting the sequence and waiting on the latch. Later the UI thread posts
// raster_task to the raster thread triggers signaling the latch(on the IO
// thread). If the raster and the platform threads are the same this results
// in a deadlock as the raster_task will never be posted to the plaform/raster
// thread that is blocked on a latch. To avoid the described deadlock, if the
// raster and the platform threads are the same, should_post_raster_task will
// be false, and then instead of posting a task to the raster thread, the ui
// thread just signals the latch and the platform/raster thread follows with
// executing raster_task.
const bool should_post_raster_task =
!task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread();

// Note:
// This is a synchronous operation because certain platforms depend on
// setup/suspension of all activities that may be interacting with the GPU in
Expand Down Expand Up @@ -881,31 +827,18 @@ void Shell::OnPlatformViewDestroyed() {

auto ui_task = [engine = engine_->GetWeakPtr(),
raster_task_runner = task_runners_.GetRasterTaskRunner(),
raster_task, should_post_raster_task, &latch]() {
raster_task]() {
if (engine) {
engine->OnOutputSurfaceDestroyed();
}
if (should_post_raster_task) {
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
} else {
// See comment on should_post_raster_task, in this case we just unblock
// the platform thread.
latch.Signal();
}
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
};

// Step 0: Post a task onto the UI thread to tell the engine that its output
// surface is about to go away.
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);

latch.Wait();
if (!should_post_raster_task) {
// See comment on should_post_raster_task, in this case the raster_task
// wasn't executed, and we just run it here as the platform thread
// is the raster thread.
raster_task();
latch.Wait();
}
}

// |PlatformView::Delegate|
Expand Down
22 changes: 1 addition & 21 deletions shell/common/shell_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,26 +345,6 @@ TEST_F(ShellTest, InitializeWithDisabledGpu) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
}

TEST_F(ShellTest, InitializeWithGPUAndPlatformThreadsTheSame) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
Settings settings = CreateSettingsForFixture();
ThreadHost thread_host(
"io.flutter.test." + GetCurrentTestName() + ".",
ThreadHost::Type::Platform | ThreadHost::Type::IO | ThreadHost::Type::UI);
TaskRunners task_runners(
"test",
thread_host.platform_thread->GetTaskRunner(), // platform
thread_host.platform_thread->GetTaskRunner(), // raster
thread_host.ui_thread->GetTaskRunner(), // ui
thread_host.io_thread->GetTaskRunner() // io
);
auto shell = CreateShell(std::move(settings), std::move(task_runners));
ASSERT_TRUE(DartVMRef::IsInstanceRunning());
ASSERT_TRUE(ValidateShell(shell.get()));
DestroyShell(std::move(shell), std::move(task_runners));
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
}

TEST_F(ShellTest, FixturesAreFunctional) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
auto settings = CreateSettingsForFixture();
Expand Down Expand Up @@ -1097,7 +1077,7 @@ TEST_F(ShellTest,
TaskRunners task_runners(
"test",
thread_host.platform_thread->GetTaskRunner(), // platform
thread_host.platform_thread->GetTaskRunner(), // raster
thread_host.raster_thread->GetTaskRunner(), // raster
thread_host.ui_thread->GetTaskRunner(), // ui
thread_host.io_thread->GetTaskRunner() // io
);
Expand Down
79 changes: 0 additions & 79 deletions shell/platform/embedder/tests/embedder_unittests_gl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1719,85 +1719,6 @@ TEST_F(EmbedderTest, CanCreateEmbedderWithCustomRenderTaskRunner) {
}
}

//------------------------------------------------------------------------------
/// Asserts that the render task runner can be the same as the platform task
/// runner.
///
TEST_F(EmbedderTest,
CanCreateEmbedderWithCustomRenderTaskRunnerTheSameAsPlatformTaskRunner) {
// A new thread needs to be created for the platform thread because the test
// can't wait for assertions to be completed on the same thread that services
// platform task runner tasks.
auto platform_task_runner = CreateNewThread("platform_thread");

static std::mutex engine_mutex;
static UniqueEngine engine;
fml::AutoResetWaitableEvent task_latch;
bool task_executed = false;
EmbedderTestTaskRunner common_task_runner(
platform_task_runner, [&](FlutterTask task) {
std::scoped_lock engine_lock(engine_mutex);
if (engine.is_valid()) {
ASSERT_EQ(FlutterEngineRunTask(engine.get(), &task), kSuccess);
task_executed = true;
task_latch.Signal();
}
});

platform_task_runner->PostTask([&]() {
EmbedderConfigBuilder builder(
GetEmbedderContext(EmbedderTestContextType::kOpenGLContext));
builder.SetDartEntrypoint("can_render_scene_without_custom_compositor");
builder.SetOpenGLRendererConfig(SkISize::Make(800, 600));
builder.SetRenderTaskRunner(
&common_task_runner.GetFlutterTaskRunnerDescription());
builder.SetPlatformTaskRunner(
&common_task_runner.GetFlutterTaskRunnerDescription());

{
std::scoped_lock lock(engine_mutex);
engine = builder.InitializeEngine();
}

ASSERT_EQ(FlutterEngineRunInitialized(engine.get()), kSuccess);

ASSERT_TRUE(engine.is_valid());

FlutterWindowMetricsEvent event = {};
event.struct_size = sizeof(event);
event.width = 800;
event.height = 600;
event.pixel_ratio = 1.0;
ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event),
kSuccess);
});

task_latch.Wait();

// Don't use the task latch because that may be called multiple time
// (including during the shutdown process).
fml::AutoResetWaitableEvent shutdown_latch;

platform_task_runner->PostTask([&]() {
ASSERT_TRUE(task_executed);
ASSERT_EQ(FlutterEngineDeinitialize(engine.get()), kSuccess);

{
std::scoped_lock engine_lock(engine_mutex);
engine.reset();
}
shutdown_latch.Signal();
});

shutdown_latch.Wait();

{
std::scoped_lock engine_lock(engine_mutex);
// Engine should have been killed by this point.
ASSERT_FALSE(engine.is_valid());
}
}

TEST_F(EmbedderTest,
CompositorMustBeAbleToRenderKnownScenePixelRatioOnSurface) {
auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext);
Expand Down