Added mutex to the pending gpu tasks deque. (flutter/engine#55748)

b/371513051

The tasks are already capturing their target thread. I had just missed
guarding the deque that is storing the tasks. This has been seen in the
flaking of the newly added `ShellTest.EncodeImageRetryOverflows`.

Example:
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8734621687934134433/+/u/test:_Host_Tests_for_host_debug_unopt/stdout

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
This commit is contained in:
gaaclarke 2024-10-08 13:19:42 -07:00 committed by GitHub
parent d75cffe14f
commit d2e73aef5b
2 changed files with 29 additions and 9 deletions

View File

@ -164,7 +164,9 @@ class ContextMTL final : public Context,
std::shared_ptr<AllocatorMTL> resource_allocator_;
std::shared_ptr<const Capabilities> device_capabilities_;
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch_;
std::deque<PendingTasks> tasks_awaiting_gpu_;
Mutex tasks_awaiting_gpu_mutex_;
std::deque<PendingTasks> tasks_awaiting_gpu_ IPLR_GUARDED_BY(
tasks_awaiting_gpu_mutex_);
std::unique_ptr<SyncSwitchObserver> sync_switch_observer_;
std::shared_ptr<CommandQueue> command_queue_ip_;
#ifdef IMPELLER_DEBUG

View File

@ -381,21 +381,39 @@ id<MTLCommandBuffer> ContextMTL::CreateMTLCommandBuffer(
void ContextMTL::StoreTaskForGPU(const fml::closure& task,
const fml::closure& failure) {
tasks_awaiting_gpu_.push_back(PendingTasks{task, failure});
while (tasks_awaiting_gpu_.size() > kMaxTasksAwaitingGPU) {
const PendingTasks& front = tasks_awaiting_gpu_.front();
if (front.failure) {
front.failure();
std::vector<PendingTasks> failed_tasks;
{
Lock lock(tasks_awaiting_gpu_mutex_);
tasks_awaiting_gpu_.push_back(PendingTasks{task, failure});
int32_t failed_task_count =
tasks_awaiting_gpu_.size() - kMaxTasksAwaitingGPU;
if (failed_task_count > 0) {
failed_tasks.reserve(failed_task_count);
failed_tasks.insert(failed_tasks.end(),
std::make_move_iterator(tasks_awaiting_gpu_.begin()),
std::make_move_iterator(tasks_awaiting_gpu_.begin() +
failed_task_count));
tasks_awaiting_gpu_.erase(
tasks_awaiting_gpu_.begin(),
tasks_awaiting_gpu_.begin() + failed_task_count);
}
}
for (const PendingTasks& task : failed_tasks) {
if (task.failure) {
task.failure();
}
tasks_awaiting_gpu_.pop_front();
}
}
void ContextMTL::FlushTasksAwaitingGPU() {
for (const auto& task : tasks_awaiting_gpu_) {
std::deque<PendingTasks> tasks_awaiting_gpu;
{
Lock lock(tasks_awaiting_gpu_mutex_);
std::swap(tasks_awaiting_gpu, tasks_awaiting_gpu_);
}
for (const auto& task : tasks_awaiting_gpu) {
task.task();
}
tasks_awaiting_gpu_.clear();
}
ContextMTL::SyncSwitchObserver::SyncSwitchObserver(ContextMTL& parent)