Skip to content

Commit 43e8a93

Browse files
auto-submit[bot]auto-submit[bot]
andauthored
Reverts "[Impeller] Speed up vulkan startup time by re-using existing vulkan context. (flutter#166784)" (flutter#166938)
<!-- start_original_pr_link --> Reverts: flutter#166784 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jonahwilliams <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: didn't do enough, going to try something different. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jonahwilliams <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Once we've created the Vulkan context, lets just use it - creating it isn't free. See numbers. Also change AHB swapchain to lazily create images, which will also improve startup and will be added to an opt in list to test startup performance. ``` ════════════════════════════╡ ••• Raw results ••• ╞═════════════════════════════ timeToFirstFrameMicros: A: 371525.00 225155.00 B: 254054.00 254668.00 timeToFirstFrameRasterizedMicros: A: 781015.00 555791.00 B: 527163.00 526827.00 ═════════════════════════╡ ••• Final A/B results ••• ╞══════════════════════════ Score Average A (noise) Average B (noise) Speed-up timeToFirstFrameMicros 298340.00 (24.53%) 254361.00 (0.12%) 1.17x timeToFirstFrameRasterizedMicros 668403.00 (16.85%) 526995.00 (0.03%) 1.27x ``` <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
1 parent 430c526 commit 43e8a93

16 files changed

Lines changed: 128 additions & 125 deletions

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.cc

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,11 @@ std::shared_ptr<AHBSwapchainImplVK> AHBSwapchainImplVK::Create(
6868
std::weak_ptr<android::SurfaceControl> surface_control,
6969
const CreateTransactionCB& cb,
7070
const ISize& size,
71-
bool enable_msaa) {
72-
auto impl = std::shared_ptr<AHBSwapchainImplVK>(new AHBSwapchainImplVK(
73-
context, std::move(surface_control), cb, size, enable_msaa));
71+
bool enable_msaa,
72+
size_t swapchain_image_count) {
73+
auto impl = std::shared_ptr<AHBSwapchainImplVK>(
74+
new AHBSwapchainImplVK(context, std::move(surface_control), cb, size,
75+
enable_msaa, swapchain_image_count));
7476
return impl->IsValid() ? impl : nullptr;
7577
}
7678

@@ -79,10 +81,12 @@ AHBSwapchainImplVK::AHBSwapchainImplVK(
7981
std::weak_ptr<android::SurfaceControl> surface_control,
8082
const CreateTransactionCB& cb,
8183
const ISize& size,
82-
bool enable_msaa)
84+
bool enable_msaa,
85+
size_t swapchain_image_count)
8386
: surface_control_(std::move(surface_control)), cb_(cb) {
8487
desc_ = android::HardwareBufferDescriptor::MakeForSwapchainImage(size);
85-
pool_ = std::make_shared<AHBTexturePoolVK>(context, desc_);
88+
pool_ =
89+
std::make_shared<AHBTexturePoolVK>(context, desc_, swapchain_image_count);
8690
if (!pool_->IsValid()) {
8791
return;
8892
}

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ class AHBSwapchainImplVK final
7373
std::weak_ptr<android::SurfaceControl> surface_control,
7474
const CreateTransactionCB& cb,
7575
const ISize& size,
76-
bool enable_msaa);
76+
bool enable_msaa,
77+
size_t swapchain_image_count);
7778

7879
~AHBSwapchainImplVK();
7980

@@ -136,7 +137,8 @@ class AHBSwapchainImplVK final
136137
std::weak_ptr<android::SurfaceControl> surface_control,
137138
const CreateTransactionCB& cb,
138139
const ISize& size,
139-
bool enable_msaa);
140+
bool enable_msaa,
141+
size_t swapchain_image_count);
140142

141143
bool Present(const std::shared_ptr<AHBTextureSourceVK>& texture);
142144

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,27 @@ bool AHBSwapchainVK::IsAvailableOnPlatform() {
2020
AHBSwapchainVK::AHBSwapchainVK(const std::shared_ptr<Context>& context,
2121
ANativeWindow* window,
2222
const CreateTransactionCB& cb,
23+
const vk::UniqueSurfaceKHR& surface,
2324
const ISize& size,
2425
bool enable_msaa)
2526
: context_(context),
2627
surface_control_(
2728
std::make_shared<android::SurfaceControl>(window, "ImpellerSurface")),
2829
enable_msaa_(enable_msaa),
2930
cb_(cb) {
31+
const auto [caps_result, surface_caps] =
32+
ContextVK::Cast(*context).GetPhysicalDevice().getSurfaceCapabilitiesKHR(
33+
*surface);
34+
if (caps_result == vk::Result::eSuccess) {
35+
swapchain_image_count_ =
36+
std::clamp(surface_caps.minImageCount + 1u, // preferred image count
37+
surface_caps.minImageCount, // min count cannot be zero
38+
surface_caps.maxImageCount == 0u
39+
? surface_caps.minImageCount + 1u
40+
: surface_caps.maxImageCount // max zero means no limit
41+
);
42+
}
43+
3044
UpdateSurfaceSize(size);
3145
}
3246

@@ -66,11 +80,12 @@ void AHBSwapchainVK::UpdateSurfaceSize(const ISize& size) {
6680
return;
6781
}
6882
TRACE_EVENT0("impeller", __FUNCTION__);
69-
auto impl = AHBSwapchainImplVK::Create(context_, //
70-
surface_control_, //
71-
cb_, //
72-
size, //
73-
enable_msaa_ //
83+
auto impl = AHBSwapchainImplVK::Create(context_, //
84+
surface_control_, //
85+
cb_, //
86+
size, //
87+
enable_msaa_, //
88+
swapchain_image_count_ //
7489
);
7590
if (!impl || !impl->IsValid()) {
7691
VALIDATION_LOG << "Could not resize swapchain to size: " << size;

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ class AHBSwapchainVK final : public SwapchainVK {
5959
std::weak_ptr<Context> context_;
6060
std::shared_ptr<android::SurfaceControl> surface_control_;
6161
const bool enable_msaa_;
62+
size_t swapchain_image_count_ = 3u;
6263
CreateTransactionCB cb_;
6364
std::shared_ptr<AHBSwapchainImplVK> impl_;
6465

6566
explicit AHBSwapchainVK(const std::shared_ptr<Context>& context,
6667
ANativeWindow* window,
6768
const CreateTransactionCB& cb,
69+
const vk::UniqueSurfaceKHR& surface,
6870
const ISize& size,
6971
bool enable_msaa);
7072
};

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_texture_pool_vk.cc

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,20 @@
99
namespace impeller {
1010

1111
AHBTexturePoolVK::AHBTexturePoolVK(std::weak_ptr<Context> context,
12-
android::HardwareBufferDescriptor desc)
13-
: context_(std::move(context)), desc_(desc) {
12+
android::HardwareBufferDescriptor desc,
13+
size_t max_entries)
14+
: context_(std::move(context)), desc_(desc), max_entries_(max_entries) {
1415
if (!desc_.IsAllocatable()) {
1516
VALIDATION_LOG << "Swapchain image is not allocatable.";
1617
return;
1718
}
18-
// Create at least one swapchain image to validate the allocation
19-
// can succeed.
20-
std::shared_ptr<AHBTextureSourceVK> texture = CreateTexture();
21-
if (!texture->IsValid()) {
22-
return;
19+
for (auto i = 0u; i < max_entries_; i++) {
20+
auto texture = CreateTexture();
21+
if (!texture->IsValid()) {
22+
return;
23+
}
24+
pool_.emplace_back(std::move(texture));
2325
}
24-
pool_.emplace_back(std::move(texture));
2526
is_valid_ = true;
2627
}
2728

@@ -48,6 +49,7 @@ void AHBTexturePoolVK::Push(std::shared_ptr<AHBTextureSourceVK> texture,
4849
}
4950
Lock lock(pool_mutex_);
5051
pool_.push_back(PoolEntry{std::move(texture), std::move(render_ready_fence)});
52+
PerformGCLocked();
5153
}
5254

5355
std::shared_ptr<AHBTextureSourceVK> AHBTexturePoolVK::CreateTexture() const {
@@ -77,6 +79,21 @@ std::shared_ptr<AHBTextureSourceVK> AHBTexturePoolVK::CreateTexture() const {
7779
return ahb_texture_source;
7880
}
7981

82+
void AHBTexturePoolVK::PerformGC() {
83+
Lock lock(pool_mutex_);
84+
PerformGCLocked();
85+
}
86+
87+
void AHBTexturePoolVK::PerformGCLocked() {
88+
while (!pool_.empty() && (pool_.size() > max_entries_)) {
89+
// Buffers are pushed to the back of the queue and popped from the front.
90+
// The ones at the back should be given the most time for their fences to
91+
// signal. If we are going to get rid of textures, they might as well be the
92+
// newest ones since their fences will take the longest to signal.
93+
pool_.pop_back();
94+
}
95+
}
96+
8097
bool AHBTexturePoolVK::IsValid() const {
8198
return is_valid_;
8299
}

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_texture_pool_vk.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ class AHBTexturePoolVK {
5353
/// in the pool.
5454
///
5555
explicit AHBTexturePoolVK(std::weak_ptr<Context> context,
56-
android::HardwareBufferDescriptor desc);
56+
android::HardwareBufferDescriptor desc,
57+
size_t max_entries = 3u);
5758

5859
~AHBTexturePoolVK();
5960

@@ -96,13 +97,24 @@ class AHBTexturePoolVK {
9697
void Push(std::shared_ptr<AHBTextureSourceVK> texture,
9798
fml::UniqueFD render_ready_fence);
9899

100+
//----------------------------------------------------------------------------
101+
/// @brief Perform an explicit GC of the pool items. This happens
102+
/// implicitly when a texture source us pushed into the pool but
103+
/// one may be necessary explicitly if there is no push back into
104+
/// the pool for a long time.
105+
///
106+
void PerformGC();
107+
99108
private:
100109
const std::weak_ptr<Context> context_;
101110
const android::HardwareBufferDescriptor desc_;
111+
const size_t max_entries_;
102112
bool is_valid_ = false;
103113
Mutex pool_mutex_;
104114
std::deque<PoolEntry> pool_ IPLR_GUARDED_BY(pool_mutex_);
105115

116+
void PerformGCLocked() IPLR_REQUIRES(pool_mutex_);
117+
106118
std::shared_ptr<AHBTextureSourceVK> CreateTexture() const;
107119
};
108120

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ std::shared_ptr<SwapchainVK> SwapchainVK::Create(
4545
return nullptr;
4646
}
4747

48+
vk::AndroidSurfaceCreateInfoKHR surface_info;
49+
surface_info.setWindow(window.GetHandle());
50+
auto [result, surface] =
51+
ContextVK::Cast(*context).GetInstance().createAndroidSurfaceKHRUnique(
52+
surface_info);
53+
if (result != vk::Result::eSuccess) {
54+
VALIDATION_LOG << "Could not create KHR Android Surface: "
55+
<< vk::to_string(result);
56+
return nullptr;
57+
}
58+
4859
// Use AHB Swapchains if they are opted in.
4960
if (ContextVK::Cast(*context).GetShouldEnableSurfaceControlSwapchain() &&
5061
AHBSwapchainVK::IsAvailableOnPlatform() &&
@@ -54,6 +65,7 @@ std::shared_ptr<SwapchainVK> SwapchainVK::Create(
5465
context, //
5566
window.GetHandle(), //
5667
cb, //
68+
surface, //
5769
window.GetSize(), //
5870
enable_msaa //
5971
));
@@ -66,17 +78,6 @@ std::shared_ptr<SwapchainVK> SwapchainVK::Create(
6678
}
6779
}
6880

69-
vk::AndroidSurfaceCreateInfoKHR surface_info;
70-
surface_info.setWindow(window.GetHandle());
71-
auto [result, surface] =
72-
ContextVK::Cast(*context).GetInstance().createAndroidSurfaceKHRUnique(
73-
surface_info);
74-
if (result != vk::Result::eSuccess) {
75-
VALIDATION_LOG << "Could not create KHR Android Surface: "
76-
<< vk::to_string(result);
77-
return nullptr;
78-
}
79-
8081
// Fallback to KHR swapchains if AHB swapchains aren't available.
8182
return Create(context, std::move(surface), window.GetSize(), enable_msaa);
8283
}

engine/src/flutter/shell/platform/android/android_shell_holder.cc

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include "flutter/shell/common/rasterizer.h"
2222
#include "flutter/shell/common/run_configuration.h"
2323
#include "flutter/shell/common/thread_host.h"
24-
#include "flutter/shell/platform/android/android_context_vk_impeller.h"
2524
#include "flutter/shell/platform/android/android_display.h"
2625
#include "flutter/shell/platform/android/android_image_generator.h"
2726
#include "flutter/shell/platform/android/android_rendering_selector.h"
@@ -83,8 +82,7 @@ static PlatformData GetDefaultPlatformData() {
8382
AndroidShellHolder::AndroidShellHolder(
8483
const flutter::Settings& settings,
8584
std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
86-
AndroidRenderingAPI android_rendering_api,
87-
std::unique_ptr<AndroidContextVKImpeller> android_vk_context)
85+
AndroidRenderingAPI android_rendering_api)
8886
: settings_(settings),
8987
jni_facade_(jni_facade),
9088
android_rendering_api_(android_rendering_api) {
@@ -115,18 +113,14 @@ AndroidShellHolder::AndroidShellHolder(
115113

116114
fml::WeakPtr<PlatformViewAndroid> weak_platform_view;
117115
AndroidRenderingAPI rendering_api = android_rendering_api_;
118-
std::shared_ptr<AndroidContextVKImpeller> shared_vk_context =
119-
std::move(android_vk_context);
120116
Shell::CreateCallback<PlatformView> on_create_platform_view =
121-
[&jni_facade, &weak_platform_view, rendering_api,
122-
shared_vk_context](Shell& shell) {
117+
[&jni_facade, &weak_platform_view, rendering_api](Shell& shell) {
123118
std::unique_ptr<PlatformViewAndroid> platform_view_android;
124119
platform_view_android = std::make_unique<PlatformViewAndroid>(
125120
shell, // delegate
126121
shell.GetTaskRunners(), // task runners
127122
jni_facade, // JNI interop
128-
rendering_api, // rendering API
129-
shared_vk_context // android vk
123+
rendering_api // rendering API
130124
);
131125
weak_platform_view = platform_view_android->GetWeakPtr();
132126
return platform_view_android;

engine/src/flutter/shell/platform/android/android_shell_holder.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "flutter/shell/platform/android/apk_asset_provider.h"
1616
#include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
1717
#include "flutter/shell/platform/android/platform_view_android.h"
18-
#include "shell/platform/android/android_context_vk_impeller.h"
1918

2019
namespace flutter {
2120

@@ -38,11 +37,9 @@ namespace flutter {
3837
///
3938
class AndroidShellHolder {
4039
public:
41-
AndroidShellHolder(
42-
const flutter::Settings& settings,
43-
std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
44-
AndroidRenderingAPI android_rendering_api,
45-
std::unique_ptr<AndroidContextVKImpeller> android_vk_context = nullptr);
40+
AndroidShellHolder(const flutter::Settings& settings,
41+
std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
42+
AndroidRenderingAPI android_rendering_api);
4643

4744
~AndroidShellHolder();
4845

engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ TEST(AndroidShellHolder, Create) {
146146
settings.enable_software_rendering = false;
147147
auto jni = std::make_shared<MockPlatformViewAndroidJNI>();
148148
auto holder = std::make_unique<AndroidShellHolder>(
149-
settings, jni, AndroidRenderingAPI::kImpellerOpenGLES, nullptr);
149+
settings, jni, AndroidRenderingAPI::kImpellerOpenGLES);
150150
EXPECT_NE(holder.get(), nullptr);
151151
EXPECT_TRUE(holder->IsValid());
152152
EXPECT_NE(holder->GetPlatformView().get(), nullptr);
@@ -160,7 +160,7 @@ TEST(AndroidShellHolder, HandlePlatformMessage) {
160160
settings.enable_software_rendering = false;
161161
auto jni = std::make_shared<MockPlatformViewAndroidJNI>();
162162
auto holder = std::make_unique<AndroidShellHolder>(
163-
settings, jni, AndroidRenderingAPI::kImpellerOpenGLES, nullptr);
163+
settings, jni, AndroidRenderingAPI::kImpellerOpenGLES);
164164
EXPECT_NE(holder.get(), nullptr);
165165
EXPECT_TRUE(holder->IsValid());
166166
EXPECT_NE(holder->GetPlatformView().get(), nullptr);

0 commit comments

Comments
 (0)