[Impeller] when creating new pipeline variant block on current thread, re-persist dirty pipeline cache. (#52375)
Current diagram of how PSO variants are created (after first bootstrap)
```
Raster thread. Worker Thread
1. Check Pipeline Cache
2. Pipeline Cache Empty
3. Create Promise
4. Spawn Worker -> Compile Pipeline
5. Block On Future. ..
6.
7. Resolve <- Complete Future
```
This is a serialized workload due to blocking on future completion. But performing it on two threads means that we're also adding in the somewhat random cost of thread scheduling and or getting deprioritized.
Instead when creating variants, block on the current thread. Now if we knew all the variants we need to create for a frame ahead of time, we could spawn one or more works to creat them more quickly. This would require more work though.
While I'm at it. Update the pipeline cache to persist every 50 frames, if it has added a new shader.
part of https://github.com/flutter/flutter/issues/129660
diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h
index 9face9f..0d748f3 100644
--- a/impeller/entity/contents/content_context.h
+++ b/impeller/entity/contents/content_context.h
@@ -1000,8 +1000,8 @@
}
auto variant_future = pipeline->CreateVariant(
- [&opts, variants_count =
- container.GetPipelineCount()](PipelineDescriptor& desc) {
+ /*async=*/false, [&opts, variants_count = container.GetPipelineCount()](
+ PipelineDescriptor& desc) {
opts.ApplyToPipelineDescriptor(desc);
desc.SetLabel(
SPrintF("%s V#%zu", desc.GetLabel().c_str(), variants_count));
diff --git a/impeller/entity/contents/content_context_unittests.cc b/impeller/entity/contents/content_context_unittests.cc
index 1736397..1063b06 100644
--- a/impeller/entity/contents/content_context_unittests.cc
+++ b/impeller/entity/contents/content_context_unittests.cc
@@ -122,8 +122,8 @@
bool IsValid() const override { return true; }
- PipelineFuture<PipelineDescriptor> GetPipeline(
- PipelineDescriptor descriptor) override {
+ PipelineFuture<PipelineDescriptor> GetPipeline(PipelineDescriptor descriptor,
+ bool async) override {
auto pipeline =
std::make_shared<FakePipeline>(weak_from_this(), descriptor);
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>> promise;
@@ -136,7 +136,8 @@
}
PipelineFuture<ComputePipelineDescriptor> GetPipeline(
- ComputePipelineDescriptor descriptor) override {
+ ComputePipelineDescriptor descriptor,
+ bool async) override {
auto pipeline =
std::make_shared<FakeComputePipeline>(weak_from_this(), descriptor);
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>> promise;
diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc
index 9077bf6..d28f64e 100644
--- a/impeller/entity/contents/runtime_effect_contents.cc
+++ b/impeller/entity/contents/runtime_effect_contents.cc
@@ -75,7 +75,8 @@
ContentContextOptions options;
options.color_attachment_pixel_format =
renderer.GetContext()->GetCapabilities()->GetDefaultColorFormat();
- return !!CreatePipeline(renderer, options);
+ CreatePipeline(renderer, options, /*async=*/true);
+ return true;
}
bool RuntimeEffectContents::RegisterShader(
@@ -134,7 +135,8 @@
std::shared_ptr<Pipeline<PipelineDescriptor>>
RuntimeEffectContents::CreatePipeline(const ContentContext& renderer,
- ContentContextOptions options) const {
+ ContentContextOptions options,
+ bool async) const {
const std::shared_ptr<Context>& context = renderer.GetContext();
const std::shared_ptr<ShaderLibrary>& library = context->GetShaderLibrary();
const std::shared_ptr<const Capabilities>& caps = context->GetCapabilities();
@@ -169,7 +171,12 @@
desc.SetDepthPixelFormat(stencil_attachment_format);
options.ApplyToPipelineDescriptor(desc);
- auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).Get();
+ if (async) {
+ context->GetPipelineLibrary()->GetPipeline(desc, async);
+ return nullptr;
+ }
+
+ auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc, async).Get();
if (!pipeline) {
VALIDATION_LOG << "Failed to get or create runtime effect pipeline.";
return nullptr;
@@ -315,8 +322,9 @@
[&](ContentContextOptions options) {
// Pipeline creation callback for the cache handler to call.
return renderer.GetCachedRuntimeEffectPipeline(
- runtime_stage_->GetEntrypoint(), options,
- [&]() { return CreatePipeline(renderer, options); });
+ runtime_stage_->GetEntrypoint(), options, [&]() {
+ return CreatePipeline(renderer, options, /*async=*/false);
+ });
};
return ColorSourceContents::DrawGeometry<VS>(renderer, entity, pass,
diff --git a/impeller/entity/contents/runtime_effect_contents.h b/impeller/entity/contents/runtime_effect_contents.h
index 7d71ba3..3602e82 100644
--- a/impeller/entity/contents/runtime_effect_contents.h
+++ b/impeller/entity/contents/runtime_effect_contents.h
@@ -41,9 +41,12 @@
private:
bool RegisterShader(const ContentContext& renderer) const;
+ // If async is true, this will always return nullptr as pipeline creation
+ // is not blocked on.
std::shared_ptr<Pipeline<PipelineDescriptor>> CreatePipeline(
const ContentContext& renderer,
- ContentContextOptions options) const;
+ ContentContextOptions options,
+ bool async) const;
std::shared_ptr<RuntimeStage> runtime_stage_;
std::shared_ptr<std::vector<uint8_t>> uniform_data_;
diff --git a/impeller/renderer/backend/gles/pipeline_library_gles.cc b/impeller/renderer/backend/gles/pipeline_library_gles.cc
index 14e3954..b730f16 100644
--- a/impeller/renderer/backend/gles/pipeline_library_gles.cc
+++ b/impeller/renderer/backend/gles/pipeline_library_gles.cc
@@ -181,7 +181,8 @@
// |PipelineLibrary|
PipelineFuture<PipelineDescriptor> PipelineLibraryGLES::GetPipeline(
- PipelineDescriptor descriptor) {
+ PipelineDescriptor descriptor,
+ bool async) {
if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) {
return found->second;
}
@@ -258,7 +259,8 @@
// |PipelineLibrary|
PipelineFuture<ComputePipelineDescriptor> PipelineLibraryGLES::GetPipeline(
- ComputePipelineDescriptor descriptor) {
+ ComputePipelineDescriptor descriptor,
+ bool async) {
auto promise = std::make_shared<
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>>();
// TODO(dnfield): implement compute for GLES.
diff --git a/impeller/renderer/backend/gles/pipeline_library_gles.h b/impeller/renderer/backend/gles/pipeline_library_gles.h
index 410fcb9..21bd253 100644
--- a/impeller/renderer/backend/gles/pipeline_library_gles.h
+++ b/impeller/renderer/backend/gles/pipeline_library_gles.h
@@ -30,12 +30,13 @@
bool IsValid() const override;
// |PipelineLibrary|
- PipelineFuture<PipelineDescriptor> GetPipeline(
- PipelineDescriptor descriptor) override;
+ PipelineFuture<PipelineDescriptor> GetPipeline(PipelineDescriptor descriptor,
+ bool async) override;
// |PipelineLibrary|
PipelineFuture<ComputePipelineDescriptor> GetPipeline(
- ComputePipelineDescriptor descriptor) override;
+ ComputePipelineDescriptor descriptor,
+ bool async) override;
// |PipelineLibrary|
void RemovePipelinesWithEntryPoint(
diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.h b/impeller/renderer/backend/metal/pipeline_library_mtl.h
index 7b12ebb..401cbcd 100644
--- a/impeller/renderer/backend/metal/pipeline_library_mtl.h
+++ b/impeller/renderer/backend/metal/pipeline_library_mtl.h
@@ -34,12 +34,13 @@
bool IsValid() const override;
// |PipelineLibrary|
- PipelineFuture<PipelineDescriptor> GetPipeline(
- PipelineDescriptor descriptor) override;
+ PipelineFuture<PipelineDescriptor> GetPipeline(PipelineDescriptor descriptor,
+ bool async) override;
// |PipelineLibrary|
PipelineFuture<ComputePipelineDescriptor> GetPipeline(
- ComputePipelineDescriptor descriptor) override;
+ ComputePipelineDescriptor descriptor,
+ bool async) override;
// |PipelineLibrary|
void RemovePipelinesWithEntryPoint(
diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.mm b/impeller/renderer/backend/metal/pipeline_library_mtl.mm
index 177c54a..059862c 100644
--- a/impeller/renderer/backend/metal/pipeline_library_mtl.mm
+++ b/impeller/renderer/backend/metal/pipeline_library_mtl.mm
@@ -109,7 +109,8 @@
// |PipelineLibrary|
PipelineFuture<PipelineDescriptor> PipelineLibraryMTL::GetPipeline(
- PipelineDescriptor descriptor) {
+ PipelineDescriptor descriptor,
+ bool async) {
if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) {
return found->second;
}
@@ -162,7 +163,8 @@
}
PipelineFuture<ComputePipelineDescriptor> PipelineLibraryMTL::GetPipeline(
- ComputePipelineDescriptor descriptor) {
+ ComputePipelineDescriptor descriptor,
+ bool async) {
if (auto found = compute_pipelines_.find(descriptor);
found != compute_pipelines_.end()) {
return found->second;
diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
index 26ed69f..9d6fc7e 100644
--- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
+++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
@@ -157,12 +157,14 @@
// |PipelineLibrary|
PipelineFuture<PipelineDescriptor> PipelineLibraryVK::GetPipeline(
- PipelineDescriptor descriptor) {
+ PipelineDescriptor descriptor,
+ bool async) {
Lock lock(pipelines_mutex_);
if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) {
return found->second;
}
+ cache_dirty_ = true;
if (!IsValid()) {
return {
descriptor,
@@ -177,7 +179,7 @@
auto weak_this = weak_from_this();
- worker_task_runner_->PostTask([descriptor, weak_this, promise]() {
+ auto generation_task = [descriptor, weak_this, promise]() {
auto thiz = weak_this.lock();
if (!thiz) {
promise->set_value(nullptr);
@@ -191,20 +193,28 @@
PipelineLibraryVK::Cast(*thiz).device_holder_.lock(), //
weak_this //
));
- });
+ };
+
+ if (async) {
+ worker_task_runner_->PostTask(generation_task);
+ } else {
+ generation_task();
+ }
return pipeline_future;
}
// |PipelineLibrary|
PipelineFuture<ComputePipelineDescriptor> PipelineLibraryVK::GetPipeline(
- ComputePipelineDescriptor descriptor) {
+ ComputePipelineDescriptor descriptor,
+ bool async) {
Lock lock(compute_pipelines_mutex_);
if (auto found = compute_pipelines_.find(descriptor);
found != compute_pipelines_.end()) {
return found->second;
}
+ cache_dirty_ = true;
if (!IsValid()) {
return {
descriptor,
@@ -220,7 +230,7 @@
auto weak_this = weak_from_this();
- worker_task_runner_->PostTask([descriptor, weak_this, promise]() {
+ auto generation_task = [descriptor, weak_this, promise]() {
auto self = weak_this.lock();
if (!self) {
promise->set_value(nullptr);
@@ -238,7 +248,13 @@
}
promise->set_value(std::move(pipeline));
- });
+ };
+
+ if (async) {
+ worker_task_runner_->PostTask(generation_task);
+ } else {
+ generation_task();
+ }
return pipeline_future;
}
@@ -256,7 +272,11 @@
void PipelineLibraryVK::DidAcquireSurfaceFrame() {
if (++frames_acquired_ == 50u) {
- PersistPipelineCacheToDisk();
+ if (cache_dirty_) {
+ cache_dirty_ = false;
+ PersistPipelineCacheToDisk();
+ }
+ frames_acquired_ = 0;
}
}
diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h
index 0f39269..e591496 100644
--- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h
+++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h
@@ -49,6 +49,7 @@
compute_pipelines_mutex_);
std::atomic_size_t frames_acquired_ = 0u;
bool is_valid_ = false;
+ bool cache_dirty_ = false;
PipelineLibraryVK(
const std::shared_ptr<DeviceHolderVK>& device_holder,
@@ -60,12 +61,13 @@
bool IsValid() const override;
// |PipelineLibrary|
- PipelineFuture<PipelineDescriptor> GetPipeline(
- PipelineDescriptor descriptor) override;
+ PipelineFuture<PipelineDescriptor> GetPipeline(PipelineDescriptor descriptor,
+ bool async) override;
// |PipelineLibrary|
PipelineFuture<ComputePipelineDescriptor> GetPipeline(
- ComputePipelineDescriptor descriptor) override;
+ ComputePipelineDescriptor descriptor,
+ bool async) override;
// |PipelineLibrary|
void RemovePipelinesWithEntryPoint(
diff --git a/impeller/renderer/pipeline.cc b/impeller/renderer/pipeline.cc
index a71b4bd..302b62d 100644
--- a/impeller/renderer/pipeline.cc
+++ b/impeller/renderer/pipeline.cc
@@ -52,6 +52,7 @@
template <typename T>
PipelineFuture<T> Pipeline<T>::CreateVariant(
+ bool async,
std::function<void(T& desc)> descriptor_callback) const {
if (!descriptor_callback) {
return {std::nullopt,
@@ -69,7 +70,7 @@
return {desc_, RealizedFuture<std::shared_ptr<Pipeline<T>>>(nullptr)};
}
- return library->GetPipeline(std::move(copied_desc));
+ return library->GetPipeline(std::move(copied_desc), async);
}
template class Pipeline<PipelineDescriptor>;
diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h
index c6b4074..b3babc3 100644
--- a/impeller/renderer/pipeline.h
+++ b/impeller/renderer/pipeline.h
@@ -62,6 +62,7 @@
const T& GetDescriptor() const;
PipelineFuture<T> CreateVariant(
+ bool async,
std::function<void(T& desc)> descriptor_callback) const;
protected:
diff --git a/impeller/renderer/pipeline_library.cc b/impeller/renderer/pipeline_library.cc
index 816d955..ef4950b 100644
--- a/impeller/renderer/pipeline_library.cc
+++ b/impeller/renderer/pipeline_library.cc
@@ -11,9 +11,10 @@
PipelineLibrary::~PipelineLibrary() = default;
PipelineFuture<PipelineDescriptor> PipelineLibrary::GetPipeline(
- std::optional<PipelineDescriptor> descriptor) {
+ std::optional<PipelineDescriptor> descriptor,
+ bool async) {
if (descriptor.has_value()) {
- return GetPipeline(descriptor.value());
+ return GetPipeline(descriptor.value(), async);
}
auto promise = std::make_shared<
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
@@ -22,9 +23,10 @@
}
PipelineFuture<ComputePipelineDescriptor> PipelineLibrary::GetPipeline(
- std::optional<ComputePipelineDescriptor> descriptor) {
+ std::optional<ComputePipelineDescriptor> descriptor,
+ bool async) {
if (descriptor.has_value()) {
- return GetPipeline(descriptor.value());
+ return GetPipeline(descriptor.value(), async);
}
auto promise = std::make_shared<
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>>();
diff --git a/impeller/renderer/pipeline_library.h b/impeller/renderer/pipeline_library.h
index d97861f..4803a10 100644
--- a/impeller/renderer/pipeline_library.h
+++ b/impeller/renderer/pipeline_library.h
@@ -8,7 +8,6 @@
#include <optional>
#include "compute_pipeline_descriptor.h"
-#include "flutter/fml/macros.h"
#include "impeller/renderer/pipeline.h"
#include "impeller/renderer/pipeline_descriptor.h"
@@ -32,18 +31,22 @@
virtual ~PipelineLibrary();
PipelineFuture<PipelineDescriptor> GetPipeline(
- std::optional<PipelineDescriptor> descriptor);
+ std::optional<PipelineDescriptor> descriptor,
+ bool async = true);
PipelineFuture<ComputePipelineDescriptor> GetPipeline(
- std::optional<ComputePipelineDescriptor> descriptor);
+ std::optional<ComputePipelineDescriptor> descriptor,
+ bool async = true);
virtual bool IsValid() const = 0;
virtual PipelineFuture<PipelineDescriptor> GetPipeline(
- PipelineDescriptor descriptor) = 0;
+ PipelineDescriptor descriptor,
+ bool async = true) = 0;
virtual PipelineFuture<ComputePipelineDescriptor> GetPipeline(
- ComputePipelineDescriptor descriptor) = 0;
+ ComputePipelineDescriptor descriptor,
+ bool async = true) = 0;
virtual void RemovePipelinesWithEntryPoint(
std::shared_ptr<const ShaderFunction> function) = 0;
diff --git a/impeller/scene/scene_context.h b/impeller/scene/scene_context.h
index 48ff7b8..1a56ea1 100644
--- a/impeller/scene/scene_context.h
+++ b/impeller/scene/scene_context.h
@@ -96,8 +96,8 @@
FML_CHECK(prototype != variants_.end());
auto variant_future = prototype->second->WaitAndGet()->CreateVariant(
- [&context, &opts,
- variants_count = variants_.size()](PipelineDescriptor& desc) {
+ /*async=*/false, [&context, &opts, variants_count = variants_.size()](
+ PipelineDescriptor& desc) {
opts.ApplyToPipelineDescriptor(*context.GetCapabilities(), desc);
desc.SetLabel(
SPrintF("%s V#%zu", desc.GetLabel().c_str(), variants_count));
