Smaller reactor handle locks (flutter/engine#56768)
This reduces the amount of work that is happening inside the read/write mutex for tracking handles in the Reactor. 1) Now gl cleanup calls happen outside of the write lock 1) Now setting gl debug texts are out of the write lock 1) Now creating objects with CreateHandle are out of the write lock Jonah was seeing considerable time in the rwmutex, this should alleviate some of that.  issue: https://github.com/flutter/flutter/issues/159177 tests: no functional change, just performance ## 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:
parent
e0084fe36b
commit
b23720bf68
@ -203,7 +203,6 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
|
||||
if (new_handle.IsDead()) {
|
||||
return HandleGLES::DeadHandle();
|
||||
}
|
||||
WriterLock handles_lock(handles_mutex_);
|
||||
|
||||
std::optional<ReactorGLES::GLStorage> gl_handle;
|
||||
if (external_handle != GL_NONE) {
|
||||
@ -211,6 +210,8 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
|
||||
} else if (CanReactOnCurrentThread()) {
|
||||
gl_handle = CreateGLHandle(GetProcTable(), type);
|
||||
}
|
||||
|
||||
WriterLock handles_lock(handles_mutex_);
|
||||
handles_[new_handle] = LiveHandle{gl_handle};
|
||||
return new_handle;
|
||||
}
|
||||
@ -266,41 +267,56 @@ bool ReactorGLES::ReactOnce() {
|
||||
bool ReactorGLES::ConsolidateHandles() {
|
||||
TRACE_EVENT0("impeller", __FUNCTION__);
|
||||
const auto& gl = GetProcTable();
|
||||
WriterLock handles_lock(handles_mutex_);
|
||||
std::vector<HandleGLES> handles_to_delete;
|
||||
for (auto& handle : handles_) {
|
||||
// Collect dead handles.
|
||||
if (handle.second.pending_collection) {
|
||||
// This could be false if the handle was created and collected without
|
||||
// use. We still need to get rid of map entry.
|
||||
if (handle.second.name.has_value()) {
|
||||
CollectGLHandle(gl, handle.first.type, handle.second.name.value());
|
||||
std::vector<std::tuple<HandleGLES, std::optional<GLStorage>>>
|
||||
handles_to_delete;
|
||||
std::vector<std::tuple<DebugResourceType, GLint, std::string>>
|
||||
handles_to_name;
|
||||
{
|
||||
WriterLock handles_lock(handles_mutex_);
|
||||
for (auto& handle : handles_) {
|
||||
// Collect dead handles.
|
||||
if (handle.second.pending_collection) {
|
||||
handles_to_delete.push_back(
|
||||
std::make_tuple(handle.first, handle.second.name));
|
||||
continue;
|
||||
}
|
||||
handles_to_delete.push_back(handle.first);
|
||||
continue;
|
||||
}
|
||||
// Create live handles.
|
||||
if (!handle.second.name.has_value()) {
|
||||
auto gl_handle = CreateGLHandle(gl, handle.first.type);
|
||||
if (!gl_handle) {
|
||||
VALIDATION_LOG << "Could not create GL handle.";
|
||||
return false;
|
||||
// Create live handles.
|
||||
if (!handle.second.name.has_value()) {
|
||||
auto gl_handle = CreateGLHandle(gl, handle.first.type);
|
||||
if (!gl_handle) {
|
||||
VALIDATION_LOG << "Could not create GL handle.";
|
||||
return false;
|
||||
}
|
||||
handle.second.name = gl_handle;
|
||||
}
|
||||
handle.second.name = gl_handle;
|
||||
}
|
||||
// Set pending debug labels.
|
||||
if (handle.second.pending_debug_label.has_value() &&
|
||||
handle.first.type != HandleType::kFence) {
|
||||
if (gl.SetDebugLabel(ToDebugResourceType(handle.first.type),
|
||||
handle.second.name.value().handle,
|
||||
handle.second.pending_debug_label.value())) {
|
||||
// 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(
|
||||
ToDebugResourceType(handle.first.type),
|
||||
handle.second.name.value().handle,
|
||||
std::move(handle.second.pending_debug_label.value())));
|
||||
handle.second.pending_debug_label = std::nullopt;
|
||||
}
|
||||
}
|
||||
for (const auto& handle_to_delete : handles_to_delete) {
|
||||
handles_.erase(std::get<0>(handle_to_delete));
|
||||
}
|
||||
}
|
||||
for (const auto& handle_to_delete : handles_to_delete) {
|
||||
handles_.erase(handle_to_delete);
|
||||
|
||||
for (const auto& handle : handles_to_name) {
|
||||
gl.SetDebugLabel(std::get<0>(handle), std::get<1>(handle),
|
||||
std::get<2>(handle));
|
||||
}
|
||||
for (const auto& handle : handles_to_delete) {
|
||||
const std::optional<GLStorage>& storage = std::get<1>(handle);
|
||||
// This could be false if the handle was created and collected without
|
||||
// use. We still need to get rid of map entry.
|
||||
if (storage.has_value()) {
|
||||
CollectGLHandle(gl, std::get<0>(handle).type, storage.value());
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user