-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[Impeller] Maintain a global map of each context's currently active thread-local command pools #169548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hread-local command pools The Impeller Vulkan back end creates a thread-local map of contexts to CommandPoolVK instances for each thread that uses Vulkan. This allows a thread to obtain the CommandPoolVK that is currently in use for a given context. When a context is shut down, the Vulkan resources used by each thread's local CommandPoolVK for that context must be freed. To do this, CommandPoolVK maintains a global map of CommandPoolVK instances. Prior to this PR Impeller was appending to the context's pool list in the global map each time a new CommandPoolVK was created. In the original implementation this worked because Impeller was only creating one CommandPoolVK per thread for a given context. However, CommandPoolVK later adopted a recycling scheme where each frame creates a new CommandPoolVK instance that acquires a Vulkan command pool from the CommandPoolRecyclerVK. So inserting every CommandPoolVK into the global map will cause the global map to grow unbounded. This PR changes the structure of the global map. The global map will now associate each context with a map of thread IDs to the CommandPoolVK that is currently placed in the thread's local storage. When a thread calls CommandPoolRecyclerVK::Dispose to clear its thread-local CommandPoolVK for a context, the corresponding entry is also removed from the global map. Fixes flutter#169208
jonahwilliams
approved these changes
May 28, 2025
Contributor
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
… playground tests On Vulkan the golden tests are using a single ContextVK instance that is shared among all tests. The ContextVK is never deleted by the test harness. So when the test process exits there may still be a CommandPoolVK in the thread-local map. The map and the CommandPoolVK will then be destructed when global and thread-local objects are destructed, which may be unsafe. Calling DisposeThreadLocalCachedResources forces cleanup of the thread-local map when each test case finishes.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 29, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 29, 2025
…y active thread-local command pools (flutter/flutter#169548)
This was referenced May 29, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 29, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 29, 2025
…y active thread-local command pools (flutter/flutter#169548)
This was referenced May 29, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 29, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 30, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 30, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 30, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 2, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 2, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
…y active thread-local command pools (flutter/flutter#169548)
flutteractionsbot
pushed a commit
to flutteractionsbot/flutter
that referenced
this pull request
Jun 4, 2025
…hread-local command pools (flutter#169548) The Impeller Vulkan back end creates a thread-local map of contexts to CommandPoolVK instances for each thread that uses Vulkan. This allows a thread to obtain the CommandPoolVK that is currently in use for a given context. When a context is shut down, the Vulkan resources used by each thread's local CommandPoolVK for that context must be freed. To do this, CommandPoolVK maintains a global map of CommandPoolVK instances. Prior to this PR Impeller was appending to the context's pool list in the global map each time a new CommandPoolVK was created. In the original implementation this worked because Impeller was only creating one CommandPoolVK per thread for a given context. However, CommandPoolVK later adopted a recycling scheme where each frame creates a new CommandPoolVK instance that acquires a Vulkan command pool from the CommandPoolRecyclerVK. So inserting every CommandPoolVK into the global map will cause the global map to grow unbounded. This PR changes the structure of the global map. The global map will now associate each context with a map of thread IDs to the CommandPoolVK that is currently placed in the thread's local storage. When a thread calls CommandPoolRecyclerVK::Dispose to clear its thread-local CommandPoolVK for a context, the corresponding entry is also removed from the global map. Fixes flutter#169208
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
…y active thread-local command pools (flutter/flutter#169548)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 16, 2025
…y active thread-local command pools (flutter/flutter#169548)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
cp: stable
cherry pick this pull request to stable release candidate branch
e: impeller
Impeller rendering backend issues and features requests
engine
flutter/engine related. See also e: labels.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Impeller Vulkan back end creates a thread-local map of contexts to CommandPoolVK instances for each thread that uses Vulkan. This allows a thread to obtain the CommandPoolVK that is currently in use for a given context.
When a context is shut down, the Vulkan resources used by each thread's local CommandPoolVK for that context must be freed. To do this, CommandPoolVK maintains a global map of CommandPoolVK instances.
Prior to this PR Impeller was appending to the context's pool list in the global map each time a new CommandPoolVK was created. In the original implementation this worked because Impeller was only creating one CommandPoolVK per thread for a given context.
However, CommandPoolVK later adopted a recycling scheme where each frame creates a new CommandPoolVK instance that acquires a Vulkan command pool from the CommandPoolRecyclerVK. So inserting every CommandPoolVK into the global map will cause the global map to grow unbounded.
This PR changes the structure of the global map. The global map will now associate each context with a map of thread IDs to the CommandPoolVK that is currently placed in the thread's local storage. When a thread calls CommandPoolRecyclerVK::Dispose to clear its thread-local CommandPoolVK for a context, the corresponding entry is also removed from the global map.
Fixes #169208