Skip to content

Commit

Permalink
[fuchsia] shader warmup fixes (#22439)
Browse files Browse the repository at this point in the history
This change contains a couple of changes that should have been in
github.com/flutter/engine/commit/3105db8ee856ffef281d018774d21a6164c81236
but fell through the cracks

First one lifts the initialization of the flutter::RunConfiguration so that
the asset manager gets set on the persistant cache before the shader
warmup happens. I'm not sure how this didnt end up in the first PR I
think it got mangled during merge conflict resolution. no test coverage
for that code because its in the middle of a 400 line constructor

Second one fixes a race condition that the tests dont catch because the
tests are single threaded. This change restructures the test that missed
this bug so that it would have caught that bug and will catch comparable
bugs.
  • Loading branch information
freiling committed Nov 20, 2020
1 parent 550c750 commit f2803ac
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
12 changes: 7 additions & 5 deletions shell/platform/fuchsia/flutter/engine.cc
Expand Up @@ -201,6 +201,12 @@ Engine::Engine(Delegate& delegate,
});
};

// Launch the engine in the appropriate configuration.
// Note: this initializes the Asset Manager on the global PersistantCache
// so it must be called before WarmupSkps() is called below.
auto run_configuration = flutter::RunConfiguration::InferFromSettings(
settings, task_runners.GetIOTaskRunner());

// Setup the callback that will instantiate the platform view.
flutter::Shell::CreateCallback<flutter::PlatformView>
on_create_platform_view = fml::MakeCopyable(
Expand Down Expand Up @@ -375,10 +381,6 @@ Engine::Engine(Delegate& delegate,
};
}

// Launch the engine in the appropriate configuration.
auto run_configuration = flutter::RunConfiguration::InferFromSettings(
shell_->GetSettings(), shell_->GetTaskRunners().GetIOTaskRunner());

auto on_run_failure = [weak = weak_factory_.GetWeakPtr()]() {
// The engine could have been killed by the caller right after the
// constructor was called but before it could run on the UI thread.
Expand Down Expand Up @@ -647,7 +649,7 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner,

// tell concurrent task runner to deserialize all skps available from
// the asset manager
concurrent_task_runner->PostTask([&raster_task_runner, skp_warmup_surface,
concurrent_task_runner->PostTask([raster_task_runner, skp_warmup_surface,
&surface_producer]() {
TRACE_DURATION("flutter", "DeserializeSkps");
std::vector<std::unique_ptr<fml::Mapping>> skp_mappings =
Expand Down
19 changes: 15 additions & 4 deletions shell/platform/fuchsia/flutter/tests/engine_unittests.cc
Expand Up @@ -34,14 +34,22 @@ class MockTaskRunner : public fml::BasicTaskRunner {
virtual ~MockTaskRunner() {}

void PostTask(const fml::closure& task) override {
task_count_++;
task();
outstanding_tasks_.push(task);
}

int GetTaskCount() { return task_count_; }

void Run() {
while (!outstanding_tasks_.empty()) {
outstanding_tasks_.front()();
outstanding_tasks_.pop();
task_count_++;
}
}

private:
int task_count_ = 0;
std::queue<const fml::closure> outstanding_tasks_;
};

class EngineTest : public ::testing::Test {
Expand All @@ -53,15 +61,16 @@ class EngineTest : public ::testing::Test {

fuchsia::ui::scenic::SessionPtr session_ptr;
scenic::Session session(std::move(session_ptr));
VulkanSurfaceProducer surface_producer(&session);
surface_producer_ = std::make_unique<VulkanSurfaceProducer>(&session);

Engine::WarmupSkps(&concurrent_task_runner_, &raster_task_runner_,
surface_producer);
*surface_producer_);
}

protected:
MockTaskRunner concurrent_task_runner_;
MockTaskRunner raster_task_runner_;
std::unique_ptr<VulkanSurfaceProducer> surface_producer_;
};

TEST_F(EngineTest, SkpWarmup) {
Expand Down Expand Up @@ -100,6 +109,8 @@ TEST_F(EngineTest, SkpWarmup) {
PersistentCache::GetCacheForProcess()->SetAssetManager(asset_manager);

WarmupSkps();
concurrent_task_runner_.Run();
raster_task_runner_.Run();

EXPECT_EQ(concurrent_task_runner_.GetTaskCount(), 1);
EXPECT_EQ(raster_task_runner_.GetTaskCount(), 1);
Expand Down

0 comments on commit f2803ac

Please sign in to comment.