[impeller] parallelize opengles shader compiles by gaaclarke · Pull Request #194022 · flutter/flutter

gaaclarke · 2026-10-08T21:12:28Z

Note

This PR depended on #193985 which refactored code to make way for this change.

This PR was authored in lieu of #193066 and was informed by @krll-kov's great investigative work.

design doc: https://github.com/flutter/rfc/blob/main/rfc/210.0001-parallel-gles-linking.md
issue: #191860

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

gemini-code-assist

Code Review

This pull request refactors the OpenGL ES pipeline compilation queue (PipelineCompileQueueGLES) to support multi-step compilation jobs (Start and Finish) and limits the number of concurrent active compilation jobs. It also updates PipelineLibraryGLES to split pipeline creation into start and finish phases, utilizing the new compile job structure, and adds corresponding unit tests. A critical review comment points out a deadlock risk in PerformJobEagerly when called on the reactor thread for an already active job, as the reactor thread blocks waiting for the job to finish while the job itself requires the reactor thread to process its completion.

gaaclarke · 2026-10-12T19:50:38Z

  static std::shared_ptr<PipelineCompileQueueGLES> Create(
-      std::shared_ptr<fml::BasicTaskRunner> worker_task_runner);
+      std::shared_ptr<fml::BasicTaskRunner> worker_task_runner,
+      size_t max_active_jobs = kDefaultMaxActiveJobs);


We'll get this set as a better value in a followup.

walley892

Mostly LGTM from a code perspective with a few nits.

Have you run any benchmarks locally to verify the performance gain here?

walley892 · 2026-10-13T21:40:16Z

+  if (!active_job->second.job->Start().ok()) {
+    RemoveActiveJob(active_job);
+  }


Do we want to bubble up or log the status in this case?

The only time this practically happens is when jobs are canceled because of shutdown. If we start doing some error handling we should reconsider.

walley892 · 2026-10-13T21:44:03Z

+    if (ActiveJobMap::iterator awaited_job = FindAwaitedActiveJob();
+        awaited_job != active_jobs_.end()) {
+      active_job = awaited_job;
+    } else if (active_jobs_.size() < max_active_jobs_ &&
+               !inactive_jobs_.empty()) {
+      active_job = ActivateOldestInactiveJob();
+      should_start = true;
+    } else if (!active_jobs_.empty()) {
+      active_job = active_jobs_.begin();
+    } else {
+      is_processing_ = false;
+      return;
+    }


Can you add comments to each case explaining the situation?

Someone is waiting on a job that we've already started - finish it immediately

Room for more jobs!

active job queue is full - finish the oldest

No more jobs!

walley892 · 2026-10-13T21:49:07Z

+  ActiveJobMap::iterator active_job;
+  bool should_start = false;


Nit maybe: Why have these variables when we can just

FinishActiveJob(awaited_job)

StartJob(ActivateOldestInactiveJob())

FinishActiveJob(active_jobs_.begin())

I think it makes the logic easier to follow if we inline

it's for the mutex, we want it out of scope.

walley892 · 2026-10-13T21:57:27Z

+  if (!job.Start().ok()) {
+    return;


Same comment about bubbling up or logging the error

gaaclarke · 2026-10-14T23:41:52Z

Looks like there is a race here in cleanup. I'll revisit it. It passed previously before the comment PR so was thinking we were good.

  [ RUN      ] EmbedderTest.RenderTextureWithImpellerOpenGLDestructCallback
  [IMPORTANT:flutter/shell/platform/embedder/embedder_surface_gl_impeller.cc(129)] Using the Impeller rendering backend (OpenGLES).
  [ERROR:flutter/fml/backtrace.cc(108)] Caught signal SIGABRT during program execution.
  Frame 0: 0x76e710a287f3 abort
  Frame 1: 0x618447b736b7 Unknown
  Frame 2: 0x618447d3ca14 impeller::PipelineLibraryGLES::PipelineCreation::~PipelineCreation()
  Frame 3: 0x618447d3df4d impeller::PipelineLibraryGLES::PipelineCompileJob::~PipelineCompileJob()
  Frame 4: 0x618447d37e17 std::__fl::__function::__func<>::destroy()
  Frame 5: 0x618447564f89 std::__fl::__function::__func<>::destroy_deallocate()
  Frame 6: 0x618447560d0b fml::MessageLoopImpl::FlushTasks()
  Frame 7: 0x61844756968a fml::MessageLoopLinux::Run()
  Frame 8: 0x618447560c31 fml::MessageLoopImpl::DoRun()
  Frame 9: 0x6184475676e1 std::__fl::__function::__func<>::operator()()
  Frame 10: 0x618447567323 fml::ThreadHandle::ThreadHandle()::$_0::__invoke()
  Frame 11: 0x76e710a94a83 Unknown
  Frame 12: 0x76e710b268e0 Unknown

shutdowns

gaaclarke · 2026-10-15T00:02:06Z

Ran EmbedderTest.RenderTextureWithImpellerOpenGLDestructCallback 100 times locally, it's good.

flutter/flutter@5c7e45a...a3d71d8 2026-10-16 [email protected] 0 Initialize variables in the DlComplexityPathReceiver helper (flutter/flutter#194540) 2026-10-16 [email protected] Roll Fuchsia Test Scripts from V-qQL4158Nw6L-QLi... to 2T18KC7-LDQtmQiLP... (flutter/flutter#194527) 2026-10-16 [email protected] Remove BorderMaskBlur (flutter/flutter#194508) 2026-10-16 [email protected] Roll Skia from a577264dc087 to 470b8816a261 (1 revision) (flutter/flutter#194526) 2026-10-16 [email protected] [ios] Use DFS traversal to find engines of a scene automatically (rather than manually manage the engines list) (flutter/flutter#192457) 2026-10-16 [email protected] [Skia][DisplayList] Map local bounds through the local matrix in DlLocalMatrixImageFilter (flutter/flutter#193963) 2026-10-16 [email protected] [framework] Fix avoid_setters_without_getters warnings on mixin declarations (flutter/flutter#193856) 2026-10-16 [email protected] Roll Dart SDK from 84686ea04815 to 55a734a01fb1 (3 revisions) (flutter/flutter#194522) 2026-10-16 [email protected] Roll Skia from d131b7789482 to a577264dc087 (7 revisions) (flutter/flutter#194514) 2026-10-16 [email protected] Roll Fuchsia GN SDK from PDJz3b3CL31uoLOkD... to kFdyRnojbVy0cxvRZ... (flutter/flutter#194510) 2026-10-16 [email protected] Roll GN to f9eca75ae554 (flutter/flutter#194471) 2026-10-16 [email protected] refactor(flutter_tools): migrate RunCommand and RunCommandBase to OptionDescriptor and OptionBundle (flutter/flutter#194362) 2026-10-15 [email protected] refactor(flutter_tools): finish TestCommand OptionDescriptor and OptionBundle migration (flutter/flutter#194361) 2026-10-15 [email protected] [iOS][a11y] Fix VoiceOver announcement for unfocused password fields (flutter/flutter#192851) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194477) 2026-10-15 [email protected] refactor(flutter_tools): finish DriveCommand OptionDescriptor migration (flutter/flutter#194360) 2026-10-15 [email protected] [flutter_tools] Roll package:vm_service to 15.4.0 (flutter/flutter#194467) 2026-10-15 [email protected] refactor(flutter_tools): migrate AttachCommand to OptionDescriptor and OptionBundle (flutter/flutter#194359) 2026-10-15 [email protected] [flutter_tools] Inject dependencies into update-packages and package_map (flutter/flutter#194046) 2026-10-15 [email protected] [impeller] parallelize opengles shader compiles (flutter/flutter#194022) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194469) 2026-10-15 [email protected] Restrict web dev server to loopback by default (flutter/flutter#187595) 2026-10-15 [email protected] Roll Packages from a1ed23c to da40eaf (33 revisions) (flutter/flutter#194459) 2026-10-15 [email protected] [web] Render soft hyphens on Skwasm (flutter/flutter#193850) 2026-10-15 [email protected] refactor(flutter_tools): migrate AssembleCommand to OptionDescriptor (flutter/flutter#194358) 2026-10-15 [email protected] [windows] Add OpenGL golden test against ANGLE at D3D11 feature level 10_0 (flutter/flutter#194321) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

gaaclarke · 2026-10-19T16:04:16Z

This was a 2x speedup in time to first frame rasterized on windows:

https://flutter-flutter-perf.luci.app/e/?begin=1791937795&enable_chart_tooltip=true&end=1792420739&plotSummary=true&queries=test%3Dflutter_gallery_win_desktop__start_up&requestType=0&use_titles=true

gaaclarke · 2026-10-19T16:12:28Z

Mokey tests are unaffected:

https://flutter-flutter-perf.luci.app/e?begin=1791794120&enable_chart_tooltip=true&end=1792420739&plotSummary=true&queries=test%3Dflutter_gallery__start_up&requestType=0&use_titles=true

gaaclarke · 2026-10-19T20:24:36Z

There is a regression in startup time on android in this test, @gaaclarke we should follow up and set the number of parallel workers more intelligently.

https://flutter-flutter-perf.luci.app/e/?begin=1791586458&enable_chart_tooltip=true&end=1792436444&keys=Xdbf99255cc234462bb54a7bd15960214&plotSummary=true&requestType=0&use_titles=true&xbaroffset=2810376

gaaclarke · 2026-10-19T23:19:14Z

Here is the gallery startup process for android demonstrating no change: https://flutter-flutter-perf.luci.app/e/?begin=1791586458&enable_chart_tooltip=true&end=1792436444&plotSummary=true&queries=sub_result%3DtimeToFirstFrameRasterizedMicros%26test%3Dflutter_gallery__start_up&use_titles=true

Maybe the x factor is the shaders used.

gaaclarke · 2026-10-19T23:22:27Z

Maybe the x factor is the shaders used.

Only the complex_layout is the only one testing startup with opengles forced.

@krll-kov

> [!NOTE] > This PR depended on flutter#193985 which refactored code to make way for this change. This PR was authored in lieu of flutter#193066 and was informed by @krll-kov's great investigative work. design doc: https://github.com/flutter/rfc/blob/main/rfc/210.0001-parallel-gles-linking.md issue: flutter#191860 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Oct 8, 2026

gaaclarke force-pushed the parallelize-compiles branch from 13c8a98 to 34071c7 Compare October 9, 2026 20:01

gaaclarke mentioned this pull request Oct 9, 2026

[Impeller][GLES] Link programs in parallel on ANGLE with GL_KHR_parallel_shader_compile #193066

Closed

7 tasks done

gaaclarke added 6 commits October 12, 2026 12:21

Started running shader compile jobs in parallel

ab49cad

removed PostTask from the queue

a1f78cb

created CompileJob interface

07d6d1f

started paralellizing the jobs

39a43c3

made sure eager jobs jump priority

0b1cef1

removed Finish return code

8a3b00e

gaaclarke force-pushed the parallelize-compiles branch from cae54e2 to 8a3b00e Compare October 12, 2026 19:22

gaaclarke changed the title ~~Parallelize compiles~~ [impeller] parallelize opengles shader compiles Oct 12, 2026

gaaclarke marked this pull request as ready for review October 12, 2026 19:34

gaaclarke added the CICD Run CI/CD label Oct 12, 2026

gemini-code-assist Bot reviewed Oct 12, 2026

View reviewed changes

Comment thread engine/src/flutter/impeller/renderer/backend/gles/pipeline_compile_queue_gles.cc Resolved

gaaclarke commented Oct 12, 2026

View reviewed changes

gaaclarke requested a review from walley892 October 12, 2026 19:57

This was referenced Oct 12, 2026

[Impeller] Guard the GLES pipeline map with a mutex #193746

Closed

[Impeller] GLES: PipelineLibraryGLES pipeline map is accessed from two threads without a lock (Flutter GPU) #193745

Open

tidy

885b3ee

walley892 requested changes Oct 13, 2026

View reviewed changes

evan feedback

d466ac9

gaaclarke requested a review from walley892 October 14, 2026 00:08

scalz mentioned this pull request Oct 14, 2026

[Impeller][Windows] Significantly slower startup compared to Skia #191860

Open

1 task done

made it so unfulfilled promises get set to nullptr as a result of

1bb4b8f

shutdowns

walley892 approved these changes Oct 15, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2026

auto-submit Bot added this pull request to the merge queue Oct 15, 2026

Merged via the queue into flutter:master with commit 697012a Oct 15, 2026
23 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2026

This was referenced Oct 16, 2026

Roll Flutter from 5c7e45ab4d9c to 2e53c3af41ad (25 revisions) flutter/packages#13288

Closed

Roll Flutter from 5c7e45ab4d9c to a3d71d82160a (26 revisions) flutter/packages#13290

Merged

gaaclarke mentioned this pull request Oct 19, 2026

[gles]: android startup time regressed with parallel shader compilation #194824

Closed

gaaclarke mentioned this pull request Nov 9, 2026

[Android][Impeller][GLES] ANR: platform thread blocked in flutter::SurfaceCreated while raster thread builds all ContentContext pipelines in the vendor shader compiler (Mali, PowerVR; itel/VILLAON, Android 14) #195903

Closed

		ActiveJobMap::iterator active_job;
		bool should_start = false;

Conversation

gaaclarke commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walley892 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Oct 14, 2026

Uh oh!

gaaclarke commented Oct 15, 2026

Uh oh!

Uh oh!

gaaclarke commented Oct 19, 2026

Uh oh!

gaaclarke commented Oct 19, 2026

Uh oh!

gaaclarke commented Oct 19, 2026

Uh oh!

gaaclarke commented Oct 19, 2026

Uh oh!

gaaclarke commented Oct 19, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gaaclarke commented Oct 8, 2026 •

edited

Loading