[Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan by dnfield · Pull Request #42146 · flutter/engine

dnfield · 2023-05-19T04:46:58Z

We should avoid creating multiple message loops that each have as many threads as processors on the machine.

gaaclarke

Ah, this is the issue jonah was referring to in the other PR, eh?

This looks mostly good except I wouldn't muddy DeviceHolder and follow the pattern we had previously of the clients holding onto a shared pointer to the concurrent task runner.

gaaclarke · 2023-05-19T16:02:33Z

impeller/renderer/backend/vulkan/device_holder.h

+  virtual const std::shared_ptr<fml::ConcurrentTaskRunner>
+  GetConcurrentWorkerTaskRunner() const = 0;


I don't think we should put this here. I think you can implement the sharing without changing DeviceHolder. Adding this to DeviceHolder changes the meaning of DeviceHolder and I'm not quite sure what concept it represents anymore.

Fair enough, I'll remove this and still give a shared pointer ot the runner to the pipeline.

I wanted this to help with testing but I can still test it by exposing a getter on the context anyway.

gaaclarke · 2023-05-19T18:26:11Z

impeller/renderer/backend/vulkan/device_holder.h


 #pragma once

+#include "flutter/fml/concurrent_message_loop.h"


You can remove this now.

gaaclarke · 2023-05-19T18:26:44Z

impeller/renderer/backend/vulkan/context_vk.cc

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


We don't need this anymore right?

It's used in the test.

gaaclarke

lgtm!

…or Andorid Vulkan (flutter/engine#42146)

flutter/engine@aac0919...f0f3fe7 2023-05-21 [email protected] Roll Skia from 7c7dff949a27 to 5b2005e47bf3 (1 revision) (flutter/engine#42199) 2023-05-21 [email protected] Roll Fuchsia Linux SDK from gQ989rlKAuTJHQR-C... to 88pzkUAkSKsJrNG38... (flutter/engine#42198) 2023-05-21 [email protected] Roll Fuchsia Mac SDK from 868_67npyO8nD_JCx... to JU-dKW3CQIUzhbqWE... (flutter/engine#42197) 2023-05-21 [email protected] Roll Skia from a60bfcb01af9 to 7c7dff949a27 (1 revision) (flutter/engine#42195) 2023-05-20 [email protected] Roll Fuchsia Linux SDK from c_fRDyBVZX-MwW5fS... to gQ989rlKAuTJHQR-C... (flutter/engine#42194) 2023-05-20 [email protected] Roll Skia from f3e9cb7d37fd to a60bfcb01af9 (1 revision) (flutter/engine#42193) 2023-05-20 [email protected] Roll Fuchsia Mac SDK from sfLkc5VBFU6UkljF6... to 868_67npyO8nD_JCx... (flutter/engine#42191) 2023-05-20 [email protected] Move SkSurface::MakeNull to SkSurfaces::Null (flutter/engine#42158) 2023-05-20 [email protected] Roll Fuchsia Linux SDK from TWjmvLCOnYAUgAzvT... to c_fRDyBVZX-MwW5fS... (flutter/engine#42189) 2023-05-20 [email protected] Roll Skia from b4a4782cf89d to f3e9cb7d37fd (1 revision) (flutter/engine#42188) 2023-05-20 [email protected] Roll Fuchsia Mac SDK from pwdDQgM88sqLmZczj... to sfLkc5VBFU6UkljF6... (flutter/engine#42187) 2023-05-20 [email protected] Roll Skia from 7202b405f061 to b4a4782cf89d (19 revisions) (flutter/engine#42185) 2023-05-20 [email protected] [Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan (flutter/engine#42146) 2023-05-20 [email protected] Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm (flutter/engine#42088) 2023-05-20 [email protected] [Impeller] Made other vulkan objects cleanup properly. (flutter/engine#42113) 2023-05-19 [email protected] [Impeller] Fix the issue that 'coverage_coords' is incorrectly calculated in 'FillPathGeometry::GetPositionUVBuffer' (flutter/engine#42155) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from TWjmvLCOnYAU to 88pzkUAkSKsJ fuchsia/sdk/core/mac-amd64 from pwdDQgM88sqL to JU-dKW3CQIUz If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…27292) flutter/engine@aac0919...f0f3fe7 2023-05-21 [email protected] Roll Skia from 7c7dff949a27 to 5b2005e47bf3 (1 revision) (flutter/engine#42199) 2023-05-21 [email protected] Roll Fuchsia Linux SDK from gQ989rlKAuTJHQR-C... to 88pzkUAkSKsJrNG38... (flutter/engine#42198) 2023-05-21 [email protected] Roll Fuchsia Mac SDK from 868_67npyO8nD_JCx... to JU-dKW3CQIUzhbqWE... (flutter/engine#42197) 2023-05-21 [email protected] Roll Skia from a60bfcb01af9 to 7c7dff949a27 (1 revision) (flutter/engine#42195) 2023-05-20 [email protected] Roll Fuchsia Linux SDK from c_fRDyBVZX-MwW5fS... to gQ989rlKAuTJHQR-C... (flutter/engine#42194) 2023-05-20 [email protected] Roll Skia from f3e9cb7d37fd to a60bfcb01af9 (1 revision) (flutter/engine#42193) 2023-05-20 [email protected] Roll Fuchsia Mac SDK from sfLkc5VBFU6UkljF6... to 868_67npyO8nD_JCx... (flutter/engine#42191) 2023-05-20 [email protected] Move SkSurface::MakeNull to SkSurfaces::Null (flutter/engine#42158) 2023-05-20 [email protected] Roll Fuchsia Linux SDK from TWjmvLCOnYAUgAzvT... to c_fRDyBVZX-MwW5fS... (flutter/engine#42189) 2023-05-20 [email protected] Roll Skia from b4a4782cf89d to f3e9cb7d37fd (1 revision) (flutter/engine#42188) 2023-05-20 [email protected] Roll Fuchsia Mac SDK from pwdDQgM88sqLmZczj... to sfLkc5VBFU6UkljF6... (flutter/engine#42187) 2023-05-20 [email protected] Roll Skia from 7202b405f061 to b4a4782cf89d (19 revisions) (flutter/engine#42185) 2023-05-20 [email protected] [Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan (flutter/engine#42146) 2023-05-20 [email protected] Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm (flutter/engine#42088) 2023-05-20 [email protected] [Impeller] Made other vulkan objects cleanup properly. (flutter/engine#42113) 2023-05-19 [email protected] [Impeller] Fix the issue that 'coverage_coords' is incorrectly calculated in 'FillPathGeometry::GetPositionUVBuffer' (flutter/engine#42155) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from TWjmvLCOnYAU to 88pzkUAkSKsJ fuchsia/sdk/core/mac-amd64 from pwdDQgM88sqL to JU-dKW3CQIUz If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

avoid creating multiple concurrent message loops for Andorid Vulkan

4c7a506

dnfield requested review from chinmaygarde, gaaclarke and jason-simmons May 19, 2023 04:46

flutter-dashboard bot added the platform-android label May 19, 2023

dnfield requested a review from jonahwilliams May 19, 2023 04:47

gaaclarke reviewed May 19, 2023

View reviewed changes

Drop addition to DeviceHolder, fix test to not exit before posting task

a410e7b

dnfield mentioned this pull request May 19, 2023

[Impeller] Enable RuntimeStage tests for the Vulkan backend. flutter/flutter#122823

Closed

dnfield and others added 2 commits May 19, 2023 09:55

whups

b5cd26e

Update context_vk.cc

e6323ce

dnfield requested a review from gaaclarke May 19, 2023 18:17

gaaclarke reviewed May 19, 2023

View reviewed changes

Update device_holder.h

9bc4200

gaaclarke approved these changes May 19, 2023

View reviewed changes

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2023

auto-submit bot merged commit 8f2d447 into flutter:main May 20, 2023

engine-flutter-autoroll mentioned this pull request May 20, 2023

Roll Flutter Engine from aac09195688d to f939b5f062d7 (9 revisions) flutter/flutter#127268

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 20, 2023

8f2d447 [Impeller] avoid creating multiple concurrent message loops f…

d989de9

…or Andorid Vulkan (flutter/engine#42146)

engine-flutter-autoroll mentioned this pull request May 20, 2023

Roll Flutter Engine from aac09195688d to ffd9118bc97c (10 revisions) flutter/flutter#127279

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 20, 2023

8f2d447 [Impeller] avoid creating multiple concurrent message loops f…

f4428c8

…or Andorid Vulkan (flutter/engine#42146)

engine-flutter-autoroll mentioned this pull request May 21, 2023

Roll Flutter Engine from aac09195688d to 12f9a46e0035 (12 revisions) flutter/flutter#127283

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 21, 2023

8f2d447 [Impeller] avoid creating multiple concurrent message loops f…

6d7b1f1

…or Andorid Vulkan (flutter/engine#42146)

engine-flutter-autoroll mentioned this pull request May 21, 2023

Roll Flutter Engine from aac09195688d to 5d1b713784a7 (13 revisions) flutter/flutter#127285

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 21, 2023

8f2d447 [Impeller] avoid creating multiple concurrent message loops f…

935c96a

…or Andorid Vulkan (flutter/engine#42146)

engine-flutter-autoroll mentioned this pull request May 21, 2023

Roll Flutter Engine from aac09195688d to f0f3fe729a91 (16 revisions) flutter/flutter#127292

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 21, 2023

8f2d447 [Impeller] avoid creating multiple concurrent message loops f…

e1a9960

…or Andorid Vulkan (flutter/engine#42146)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan#42146

[Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan#42146
auto-submit[bot] merged 5 commits intoflutter:mainfrom
dnfield:workers

dnfield commented May 19, 2023

Uh oh!

gaaclarke left a comment

Uh oh!

gaaclarke May 19, 2023

Uh oh!

dnfield May 19, 2023

Uh oh!

gaaclarke May 19, 2023

Uh oh!

dnfield May 19, 2023

Uh oh!

gaaclarke May 19, 2023

Uh oh!

dnfield May 19, 2023

Uh oh!

gaaclarke left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		virtual const std::shared_ptr<fml::ConcurrentTaskRunner>
		GetConcurrentWorkerTaskRunner() const = 0;


		#pragma once

		#include "flutter/fml/concurrent_message_loop.h"

Conversation

dnfield commented May 19, 2023

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke May 19, 2023

Choose a reason for hiding this comment

Uh oh!

dnfield May 19, 2023

Choose a reason for hiding this comment

Uh oh!

gaaclarke May 19, 2023

Choose a reason for hiding this comment

Uh oh!

dnfield May 19, 2023

Choose a reason for hiding this comment

Uh oh!

gaaclarke May 19, 2023

Choose a reason for hiding this comment

Uh oh!

dnfield May 19, 2023

Choose a reason for hiding this comment

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants