From 8ad87e8401ebb061b3dc903bbaa64b84d483e92c Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:35:33 -0800 Subject: [PATCH] [impeller] gles: started storing the number of handle deletions to avoid reallocation (flutter/engine#56799) I saw in profiles that we were spending a lot of time in push_back_slow_path. Reserving + emplacing removes that. test-except: no functional change, performance increase ## 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]. [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 --- .../impeller/renderer/backend/gles/reactor_gles.cc | 9 +++++++-- .../impeller/renderer/backend/gles/reactor_gles.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc index c72f0d4af8..b46505125d 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc @@ -219,6 +219,9 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) { void ReactorGLES::CollectHandle(HandleGLES handle) { WriterLock handles_lock(handles_mutex_); if (auto found = handles_.find(handle); found != handles_.end()) { + if (!found->second.pending_collection) { + handles_to_collect_count_ += 1; + } found->second.pending_collection = true; } } @@ -273,10 +276,12 @@ bool ReactorGLES::ConsolidateHandles() { handles_to_name; { WriterLock handles_lock(handles_mutex_); + handles_to_delete.reserve(handles_to_collect_count_); + handles_to_collect_count_ = 0; for (auto& handle : handles_) { // Collect dead handles. if (handle.second.pending_collection) { - handles_to_delete.push_back( + handles_to_delete.emplace_back( std::make_tuple(handle.first, handle.second.name)); continue; } @@ -292,7 +297,7 @@ bool ReactorGLES::ConsolidateHandles() { // Set pending debug labels. if (handle.second.pending_debug_label.has_value() && handle.first.type != HandleType::kFence) { - handles_to_name.push_back(std::make_tuple( + handles_to_name.emplace_back(std::make_tuple( ToDebugResourceType(handle.first.type), handle.second.name.value().handle, std::move(handle.second.pending_debug_label.value()))); diff --git a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h index 88d77ccffe..37816400ee 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h @@ -284,6 +284,7 @@ class ReactorGLES { HandleGLES::Equal>; mutable RWMutex handles_mutex_; LiveHandles handles_ IPLR_GUARDED_BY(handles_mutex_); + int32_t handles_to_collect_count_ IPLR_GUARDED_BY(handles_mutex_) = 0; mutable Mutex workers_mutex_; mutable std::map> workers_ IPLR_GUARDED_BY(