Started caching HandleGLES's hash and made them immutable (#56800)
https://github.com/flutter/flutter/issues/159177
test exempt: no functional change, performance
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
diff --git a/impeller/renderer/backend/gles/handle_gles.h b/impeller/renderer/backend/gles/handle_gles.h
index 3e7b44f..80220f5 100644
--- a/impeller/renderer/backend/gles/handle_gles.h
+++ b/impeller/renderer/backend/gles/handle_gles.h
@@ -34,10 +34,8 @@
/// OpenGL object handles, these handles can be collected on any
/// thread as long as their destruction is scheduled in a reactor.
///
-struct HandleGLES {
- HandleType type = HandleType::kUnknown;
- std::optional<UniqueID> name;
-
+class HandleGLES {
+ public:
//----------------------------------------------------------------------------
/// @brief Creates a dead handle.
///
@@ -52,7 +50,7 @@
///
/// @return True if dead, False otherwise.
///
- constexpr bool IsDead() const { return !name.has_value(); }
+ constexpr bool IsDead() const { return !name_.has_value(); }
//----------------------------------------------------------------------------
/// @brief Get the hash value of this handle. Handles can be used as map
@@ -60,9 +58,7 @@
///
struct Hash {
std::size_t operator()(const HandleGLES& handle) const {
- return fml::HashCombine(
- std::underlying_type_t<decltype(handle.type)>(handle.type),
- handle.name);
+ return handle.GetHash();
}
};
@@ -71,17 +67,32 @@
///
struct Equal {
bool operator()(const HandleGLES& lhs, const HandleGLES& rhs) const {
- return lhs.type == rhs.type && lhs.name == rhs.name;
+ return lhs.type_ == rhs.type_ && lhs.name_ == rhs.name_;
}
};
+ HandleType GetType() const { return type_; }
+ const std::optional<UniqueID>& GetName() const { return name_; }
+ std::size_t GetHash() const { return hash_; }
+
private:
+ HandleType type_ = HandleType::kUnknown;
+ std::optional<UniqueID> name_;
+ std::size_t hash_;
+
friend class ReactorGLES;
- HandleGLES(HandleType p_type, UniqueID p_name) : type(p_type), name(p_name) {}
+ HandleGLES(HandleType p_type, UniqueID p_name)
+ : type_(p_type),
+ name_(p_name),
+ hash_(fml::HashCombine(std::underlying_type_t<decltype(p_type)>(p_type),
+ p_name)) {}
HandleGLES(HandleType p_type, std::optional<UniqueID> p_name)
- : type(p_type), name(p_name) {}
+ : type_(p_type),
+ name_(p_name),
+ hash_(fml::HashCombine(std::underlying_type_t<decltype(p_type)>(p_type),
+ p_name)) {}
static HandleGLES Create(HandleType type) {
return HandleGLES{type, UniqueID{}};
@@ -94,12 +105,13 @@
inline std::ostream& operator<<(std::ostream& out,
const impeller::HandleGLES& handle) {
- out << HandleTypeToString(handle.type) << "(";
+ out << HandleTypeToString(handle.GetType()) << "(";
if (handle.IsDead()) {
out << "DEAD";
} else {
- if (handle.name.has_value()) {
- out << handle.name.value().id;
+ const std::optional<impeller::UniqueID>& name = handle.GetName();
+ if (name.has_value()) {
+ out << name.value().id;
} else {
out << "UNNAMED";
}
diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc
index b465051..a86b211 100644
--- a/impeller/renderer/backend/gles/reactor_gles.cc
+++ b/impeller/renderer/backend/gles/reactor_gles.cc
@@ -84,7 +84,7 @@
if (CanReactOnCurrentThread()) {
for (auto& handle : handles_) {
if (handle.second.name.has_value()) {
- CollectGLHandle(*proc_table_, handle.first.type,
+ CollectGLHandle(*proc_table_, handle.first.GetType(),
handle.second.name.value());
}
}
@@ -145,7 +145,7 @@
}
std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
- if (handle.type == HandleType::kFence) {
+ if (handle.GetType() == HandleType::kFence) {
return std::nullopt;
}
std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
@@ -156,7 +156,7 @@
}
std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
- if (handle.type != HandleType::kFence) {
+ if (handle.GetType() != HandleType::kFence) {
return std::nullopt;
}
std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
@@ -287,7 +287,7 @@
}
// Create live handles.
if (!handle.second.name.has_value()) {
- auto gl_handle = CreateGLHandle(gl, handle.first.type);
+ auto gl_handle = CreateGLHandle(gl, handle.first.GetType());
if (!gl_handle) {
VALIDATION_LOG << "Could not create GL handle.";
return false;
@@ -296,9 +296,9 @@
}
// Set pending debug labels.
if (handle.second.pending_debug_label.has_value() &&
- handle.first.type != HandleType::kFence) {
+ handle.first.GetType() != HandleType::kFence) {
handles_to_name.emplace_back(std::make_tuple(
- ToDebugResourceType(handle.first.type),
+ ToDebugResourceType(handle.first.GetType()),
handle.second.name.value().handle,
std::move(handle.second.pending_debug_label.value())));
handle.second.pending_debug_label = std::nullopt;
@@ -318,7 +318,7 @@
// 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());
+ CollectGLHandle(gl, std::get<0>(handle).GetType(), storage.value());
}
}
diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h
index 3781640..881e2a9 100644
--- a/impeller/renderer/backend/gles/reactor_gles.h
+++ b/impeller/renderer/backend/gles/reactor_gles.h
@@ -278,7 +278,7 @@
// Make sure the container is one where erasing items during iteration doesn't
// invalidate other iterators.
- using LiveHandles = std::unordered_map<HandleGLES,
+ using LiveHandles = std::unordered_map<const HandleGLES,
LiveHandle,
HandleGLES::Hash,
HandleGLES::Equal>;
diff --git a/impeller/renderer/backend/gles/test/pipeline_library_gles_unittests.cc b/impeller/renderer/backend/gles/test/pipeline_library_gles_unittests.cc
index 23d54c4..34dcff5 100644
--- a/impeller/renderer/backend/gles/test/pipeline_library_gles_unittests.cc
+++ b/impeller/renderer/backend/gles/test/pipeline_library_gles_unittests.cc
@@ -38,8 +38,8 @@
// The program handles should be live and equal.
ASSERT_FALSE(pipeline_gles.GetProgramHandle().IsDead());
ASSERT_FALSE(new_pipeline_gles.GetProgramHandle().IsDead());
- ASSERT_EQ(pipeline_gles.GetProgramHandle().name.value(),
- new_pipeline_gles.GetProgramHandle().name.value());
+ ASSERT_EQ(pipeline_gles.GetProgramHandle().GetName().value(),
+ new_pipeline_gles.GetProgramHandle().GetName().value());
}
TEST_P(PipelineLibraryGLESTest, ChangingSpecConstantsCausesNewProgramObject) {
@@ -63,8 +63,8 @@
// The program handles should be live and equal.
ASSERT_FALSE(pipeline_gles.GetProgramHandle().IsDead());
ASSERT_FALSE(new_pipeline_gles.GetProgramHandle().IsDead());
- ASSERT_FALSE(pipeline_gles.GetProgramHandle().name.value() ==
- new_pipeline_gles.GetProgramHandle().name.value());
+ ASSERT_FALSE(pipeline_gles.GetProgramHandle().GetName().value() ==
+ new_pipeline_gles.GetProgramHandle().GetName().value());
}
// NOLINTEND(bugprone-unchecked-optional-access)
diff --git a/impeller/renderer/backend/gles/test/texture_gles_unittests.cc b/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
index ceafa3a..fd17972 100644
--- a/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
+++ b/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
@@ -44,7 +44,7 @@
if (!sync_fence.has_value()) {
return;
}
- EXPECT_EQ(sync_fence.value().type, HandleType::kFence);
+ EXPECT_EQ(sync_fence.value().GetType(), HandleType::kFence);
std::optional<GLsync> sync =
context_gles.GetReactor()->GetGLFence(sync_fence.value());
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
index 682e2cf..748fb5e 100644
--- a/impeller/renderer/backend/gles/texture_gles.cc
+++ b/impeller/renderer/backend/gles/texture_gles.cc
@@ -159,7 +159,7 @@
VALIDATION_LOG << "Cannot wrap a dead handle.";
return nullptr;
}
- if (external_handle.type != HandleType::kTexture) {
+ if (external_handle.GetType() != HandleType::kTexture) {
VALIDATION_LOG << "Cannot wrap a non-texture handle.";
return nullptr;
}
