firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Started caching HandleGLES's hash and made them immutable (flutter/engine#56800)

Browse Source 
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
This commit is contained in:
gaaclarke 2024-11-25 17:09:25 -08:00 committed by GitHub
parent 57b102520b
commit 6d9cdb5750
6 changed files with 40 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h
View File

@ -34,10 +34,8 @@ class ReactorGLES;
/// OpenGL object handles, these handles can be collected on any /// OpenGL object handles, these handles can be collected on any
/// thread as long as their destruction is scheduled in a reactor. /// thread as long as their destruction is scheduled in a reactor.
/// ///
struct HandleGLES { class HandleGLES {
HandleType type = HandleType::kUnknown; public:
std::optional<UniqueID> name;
//---------------------------------------------------------------------------- //----------------------------------------------------------------------------
/// @brief Creates a dead handle. /// @brief Creates a dead handle.
/// ///
@ -52,7 +50,7 @@ struct HandleGLES {
/// ///
/// @return True if dead, False otherwise. /// @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 /// @brief Get the hash value of this handle. Handles can be used as map
@ -60,9 +58,7 @@ struct HandleGLES {
/// ///
struct Hash { struct Hash {
std::size_t operator()(const HandleGLES& handle) const { std::size_t operator()(const HandleGLES& handle) const {
return fml::HashCombine( return handle.GetHash();
std::underlying_type_t<decltype(handle.type)>(handle.type),
handle.name);
} }
}; };
@ -71,17 +67,32 @@ struct HandleGLES {
/// ///
struct Equal { struct Equal {
bool operator()(const HandleGLES& lhs, const HandleGLES& rhs) const { 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: private:
HandleType type_ = HandleType::kUnknown;
std::optional<UniqueID> name_;
std::size_t hash_;
friend class ReactorGLES; 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) 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) { static HandleGLES Create(HandleType type) {
return HandleGLES{type, UniqueID{}}; return HandleGLES{type, UniqueID{}};
@ -94,12 +105,13 @@ namespace std {
inline std::ostream& operator<<(std::ostream& out, inline std::ostream& operator<<(std::ostream& out,
const impeller::HandleGLES& handle) { const impeller::HandleGLES& handle) {
out << HandleTypeToString(handle.type) << "("; out << HandleTypeToString(handle.GetType()) << "(";
if (handle.IsDead()) { if (handle.IsDead()) {
out << "DEAD"; out << "DEAD";
} else { } else {
if (handle.name.has_value()) { const std::optional<impeller::UniqueID>& name = handle.GetName();
out << handle.name.value().id; if (name.has_value()) {
out << name.value().id;
} else { } else {
out << "UNNAMED"; out << "UNNAMED";
} }

14
engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc
View File

@ -84,7 +84,7 @@ ReactorGLES::~ReactorGLES() {
if (CanReactOnCurrentThread()) { if (CanReactOnCurrentThread()) {
for (auto& handle : handles_) { for (auto& handle : handles_) {
if (handle.second.name.has_value()) { if (handle.second.name.has_value()) {
CollectGLHandle(*proc_table_, handle.first.type, CollectGLHandle(*proc_table_, handle.first.GetType(),
handle.second.name.value()); handle.second.name.value());
} }
} }
@ -145,7 +145,7 @@ std::optional<ReactorGLES::GLStorage> ReactorGLES::GetHandle(
} }
std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const { std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
if (handle.type == HandleType::kFence) { if (handle.GetType() == HandleType::kFence) {
return std::nullopt; return std::nullopt;
} }
std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle); 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 { std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
if (handle.type != HandleType::kFence) { if (handle.GetType() != HandleType::kFence) {
return std::nullopt; return std::nullopt;
} }
std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle); std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
@ -287,7 +287,7 @@ bool ReactorGLES::ConsolidateHandles() {
} }
// Create live handles. // Create live handles.
if (!handle.second.name.has_value()) { 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) { if (!gl_handle) {
VALIDATION_LOG << "Could not create GL handle."; VALIDATION_LOG << "Could not create GL handle.";
return false; return false;
@ -296,9 +296,9 @@ bool ReactorGLES::ConsolidateHandles() {
} }
// Set pending debug labels. // Set pending debug labels.
if (handle.second.pending_debug_label.has_value() && 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( handles_to_name.emplace_back(std::make_tuple(
ToDebugResourceType(handle.first.type), ToDebugResourceType(handle.first.GetType()),
handle.second.name.value().handle, handle.second.name.value().handle,
std::move(handle.second.pending_debug_label.value()))); std::move(handle.second.pending_debug_label.value())));
handle.second.pending_debug_label = std::nullopt; 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 // This could be false if the handle was created and collected without
// use. We still need to get rid of map entry. // use. We still need to get rid of map entry.
if (storage.has_value()) { if (storage.has_value()) {
CollectGLHandle(gl, std::get<0>(handle).type, storage.value()); CollectGLHandle(gl, std::get<0>(handle).GetType(), storage.value());
} }
} }

2
engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h
View File

@ -278,7 +278,7 @@ class ReactorGLES {
// Make sure the container is one where erasing items during iteration doesn't // Make sure the container is one where erasing items during iteration doesn't
// invalidate other iterators. // invalidate other iterators.
using LiveHandles = std::unordered_map<HandleGLES, using LiveHandles = std::unordered_map<const HandleGLES,
LiveHandle, LiveHandle,
HandleGLES::Hash, HandleGLES::Hash,
HandleGLES::Equal>; HandleGLES::Equal>;

8
engine/src/flutter/impeller/renderer/backend/gles/test/pipeline_library_gles_unittests.cc
View File

@ -38,8 +38,8 @@ TEST_P(PipelineLibraryGLESTest, ProgramHandlesAreReused) {
// The program handles should be live and equal. // The program handles should be live and equal.
ASSERT_FALSE(pipeline_gles.GetProgramHandle().IsDead()); ASSERT_FALSE(pipeline_gles.GetProgramHandle().IsDead());
ASSERT_FALSE(new_pipeline_gles.GetProgramHandle().IsDead()); ASSERT_FALSE(new_pipeline_gles.GetProgramHandle().IsDead());
ASSERT_EQ(pipeline_gles.GetProgramHandle().name.value(), ASSERT_EQ(pipeline_gles.GetProgramHandle().GetName().value(),
new_pipeline_gles.GetProgramHandle().name.value()); new_pipeline_gles.GetProgramHandle().GetName().value());
} }
TEST_P(PipelineLibraryGLESTest, ChangingSpecConstantsCausesNewProgramObject) { TEST_P(PipelineLibraryGLESTest, ChangingSpecConstantsCausesNewProgramObject) {
@ -63,8 +63,8 @@ TEST_P(PipelineLibraryGLESTest, ChangingSpecConstantsCausesNewProgramObject) {
// The program handles should be live and equal. // The program handles should be live and equal.
ASSERT_FALSE(pipeline_gles.GetProgramHandle().IsDead()); ASSERT_FALSE(pipeline_gles.GetProgramHandle().IsDead());
ASSERT_FALSE(new_pipeline_gles.GetProgramHandle().IsDead()); ASSERT_FALSE(new_pipeline_gles.GetProgramHandle().IsDead());
ASSERT_FALSE(pipeline_gles.GetProgramHandle().name.value() == ASSERT_FALSE(pipeline_gles.GetProgramHandle().GetName().value() ==
new_pipeline_gles.GetProgramHandle().name.value()); new_pipeline_gles.GetProgramHandle().GetName().value());
} }
// NOLINTEND(bugprone-unchecked-optional-access) // NOLINTEND(bugprone-unchecked-optional-access)

2
engine/src/flutter/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
View File

@ -44,7 +44,7 @@ TEST_P(TextureGLESTest, CanSetSyncFence) {
if (!sync_fence.has_value()) { if (!sync_fence.has_value()) {
return; return;
} }
EXPECT_EQ(sync_fence.value().type, HandleType::kFence); EXPECT_EQ(sync_fence.value().GetType(), HandleType::kFence);
std::optional<GLsync> sync = std::optional<GLsync> sync =
context_gles.GetReactor()->GetGLFence(sync_fence.value()); context_gles.GetReactor()->GetGLFence(sync_fence.value());

2
engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc
View File

@ -159,7 +159,7 @@ std::shared_ptr<TextureGLES> TextureGLES::WrapTexture(
VALIDATION_LOG << "Cannot wrap a dead handle."; VALIDATION_LOG << "Cannot wrap a dead handle.";
return nullptr; return nullptr;
} }
if (external_handle.type != HandleType::kTexture) { if (external_handle.GetType() != HandleType::kTexture) {
VALIDATION_LOG << "Cannot wrap a non-texture handle."; VALIDATION_LOG << "Cannot wrap a non-texture handle.";
return nullptr; return nullptr;
} }