Started caching HandleGLES's hash and made them immutable

impeller/renderer/backend/gles/handle_gles.h

@@ -34,10 +34,8 @@ class ReactorGLES;
 ///             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,17 +50,15 @@ struct HandleGLES {
   ///
   /// @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
   ///             keys.
   ///
   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 HandleGLES {
   ///
   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 @@ namespace std {
 
 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";
     }

impeller/renderer/backend/gles/reactor_gles.cc

@@ -84,7 +84,7 @@ ReactorGLES::~ReactorGLES() {
   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<ReactorGLES::GLStorage> ReactorGLES::GetHandle(
 }
 
 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<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
 }
 
 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 @@ bool ReactorGLES::ConsolidateHandles() {
       }
       // 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 @@ bool ReactorGLES::ConsolidateHandles() {
       }
       // 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 @@ bool ReactorGLES::ConsolidateHandles() {
     // 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());
     }
   }
 

impeller/renderer/backend/gles/reactor_gles.h

@@ -278,7 +278,7 @@ class ReactorGLES {
 
   // 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>;

impeller/renderer/backend/gles/test/pipeline_library_gles_unittests.cc

@@ -38,8 +38,8 @@ TEST_P(PipelineLibraryGLESTest, ProgramHandlesAreReused) {
   // 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 @@ TEST_P(PipelineLibraryGLESTest, ChangingSpecConstantsCausesNewProgramObject) {
   // 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)
 

impeller/renderer/backend/gles/test/texture_gles_unittests.cc

@@ -44,7 +44,7 @@ TEST_P(TextureGLESTest, CanSetSyncFence) {
   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());

impeller/renderer/backend/gles/texture_gles.cc

@@ -159,7 +159,7 @@ std::shared_ptr<TextureGLES> TextureGLES::WrapTexture(
     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;
   }