[impeller] makes UniformBindData 15% faster and adds unit test (#56844)
issue: https://github.com/flutter/flutter/issues/159177
The improvements come from using absl::flat_hash_map.
measurements with std vs absl (`host_profile_arm64`):
```
# Std
0.041124
0.0398002
0.0396546
0.0405552
0.0397742
# Absl
0.0353708
0.0336834
0.0338616
0.033050
0.0331976
```
Benchmark used
```c++
TEST(BufferBindingsGLESTest, BindUniformDataMicro) {
BufferBindingsGLES bindings;
absl::flat_hash_map<std::string, GLint> uniform_bindings;
uniform_bindings["SHADERMETADATA.FOOBAR"] = 1;
bindings.SetUniformBindings(std::move(uniform_bindings));
std::shared_ptr<MockGLES> mock_gl = MockGLES::Init();
MockAllocator allocator;
Bindings vertex_bindings;
ShaderMetadata shader_metadata = {
.name = "shader_metadata",
.members = {ShaderStructMemberMetadata{.type = ShaderType::kFloat,
.name = "foobar",
.offset = 0,
.size = sizeof(float),
.byte_length = sizeof(float)}}};
std::shared_ptr<ReactorGLES> reactor;
std::shared_ptr<Allocation> backing_store = std::make_shared<Allocation>();
ASSERT_TRUE(backing_store->Truncate(Bytes{sizeof(float)}));
DeviceBufferGLES device_buffer(DeviceBufferDescriptor{.size = sizeof(float)},
reactor, backing_store);
BufferView buffer_view(&device_buffer, Range(0, sizeof(float)));
vertex_bindings.buffers.push_back(BufferAndUniformSlot{
.slot =
ShaderUniformSlot{
.name = "foobar", .ext_res_0 = 0, .set = 0, .binding = 0},
.view = BufferResource(&shader_metadata, buffer_view)});
Bindings fragment_bindings;
int32_t count = 5'000'000;
auto start = std::chrono::high_resolution_clock::now();
for (int32_t i = 0; i < count; ++i) {
bindings.BindUniformData(mock_gl->GetProcTable(), allocator,
vertex_bindings, fragment_bindings);
if (i % 100 == 0) {
mock_gl->GetCapturedCalls();
}
}
auto end = std::chrono::high_resolution_clock::now();
auto duration =
std::chrono::duration_cast<std::chrono::microseconds>(end - start)
.count();
std::cout << "Execution time: " << duration / static_cast<double>(count)
<< " microseconds" << std::endl;
}
```
This is one of our hottest symbols on the raster thread:
<img width="1528" alt="Screenshot 2024-11-27 at 2 27 54 PM"
src="https://github.com/user-attachments/assets/a9029d6f-ee96-4612-83f1-6b69f24e6ce8">
## 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/Chatdiff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
index 955d08c..ccc34d3 100644
--- a/ci/licenses_golden/excluded_files
+++ b/ci/licenses_golden/excluded_files
@@ -182,6 +182,7 @@
../../../flutter/impeller/geometry/trig_unittests.cc
../../../flutter/impeller/golden_tests/README.md
../../../flutter/impeller/playground
+../../../flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
../../../flutter/impeller/renderer/backend/gles/test
../../../flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/metal/texture_mtl_unittests.mm
diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn
index f0b5326..13ffcd8 100644
--- a/impeller/renderer/backend/gles/BUILD.gn
+++ b/impeller/renderer/backend/gles/BUILD.gn
@@ -14,6 +14,7 @@
impeller_component("gles_unittests") {
testonly = true
sources = [
+ "buffer_bindings_gles_unittests.cc",
"test/capabilities_unittests.cc",
"test/formats_gles_unittests.cc",
"test/gpu_tracer_gles_unittests.cc",
diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc
index 9deb66f..9ad2339 100644
--- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc
+++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc
@@ -180,16 +180,15 @@
}
bool BufferBindingsGLES::BindUniformData(const ProcTableGLES& gl,
- Allocator& transients_allocator,
const Bindings& vertex_bindings,
const Bindings& fragment_bindings) {
for (const auto& buffer : vertex_bindings.buffers) {
- if (!BindUniformBuffer(gl, transients_allocator, buffer.view)) {
+ if (!BindUniformBuffer(gl, buffer.view)) {
return false;
}
}
for (const auto& buffer : fragment_bindings.buffers) {
- if (!BindUniformBuffer(gl, transients_allocator, buffer.view)) {
+ if (!BindUniformBuffer(gl, buffer.view)) {
return false;
}
}
@@ -262,7 +261,7 @@
size_t element_count = member.array_elements.value_or(1);
const std::string member_key =
CreateUniformMemberKey(metadata->name, member.name, element_count > 1);
- const std::unordered_map<std::string, GLint>::iterator computed_location =
+ const absl::flat_hash_map<std::string, GLint>::iterator computed_location =
uniform_locations_.find(member_key);
if (computed_location == uniform_locations_.end()) {
// Uniform was not active.
@@ -275,7 +274,6 @@
}
bool BufferBindingsGLES::BindUniformBuffer(const ProcTableGLES& gl,
- Allocator& transients_allocator,
const BufferResource& buffer) {
const ShaderMetadata* metadata = buffer.GetMetadata();
const DeviceBuffer* device_buffer = buffer.resource.GetBuffer();
diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.h b/impeller/renderer/backend/gles/buffer_bindings_gles.h
index 65642d8..07bae42 100644
--- a/impeller/renderer/backend/gles/buffer_bindings_gles.h
+++ b/impeller/renderer/backend/gles/buffer_bindings_gles.h
@@ -8,6 +8,7 @@
#include <unordered_map>
#include <vector>
+#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h"
#include "impeller/core/shader_types.h"
#include "impeller/renderer/backend/gles/gles.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"
@@ -15,6 +16,10 @@
namespace impeller {
+namespace testing {
+FML_TEST_CLASS(BufferBindingsGLESTest, BindUniformData);
+} // namespace testing
+
//------------------------------------------------------------------------------
/// @brief Sets up stage bindings for single draw call in the OpenGLES
/// backend.
@@ -37,13 +42,13 @@
size_t vertex_offset);
bool BindUniformData(const ProcTableGLES& gl,
- Allocator& transients_allocator,
const Bindings& vertex_bindings,
const Bindings& fragment_bindings);
bool UnbindVertexAttributes(const ProcTableGLES& gl);
private:
+ FML_FRIEND_TEST(testing::BufferBindingsGLESTest, BindUniformData);
//----------------------------------------------------------------------------
/// @brief The arguments to glVertexAttribPointer.
///
@@ -57,9 +62,9 @@
};
std::vector<std::vector<VertexAttribPointer>> vertex_attrib_arrays_;
- std::unordered_map<std::string, GLint> uniform_locations_;
+ absl::flat_hash_map<std::string, GLint> uniform_locations_;
- using BindingMap = std::unordered_map<std::string, std::vector<GLint>>;
+ using BindingMap = absl::flat_hash_map<std::string, std::vector<GLint>>;
BindingMap binding_map_ = {};
GLuint vertex_array_object_ = 0;
@@ -68,9 +73,7 @@
GLint ComputeTextureLocation(const ShaderMetadata* metadata);
- bool BindUniformBuffer(const ProcTableGLES& gl,
- Allocator& transients_allocator,
- const BufferResource& buffer);
+ bool BindUniformBuffer(const ProcTableGLES& gl, const BufferResource& buffer);
std::optional<size_t> BindTextures(const ProcTableGLES& gl,
const Bindings& bindings,
@@ -80,6 +83,12 @@
BufferBindingsGLES(const BufferBindingsGLES&) = delete;
BufferBindingsGLES& operator=(const BufferBindingsGLES&) = delete;
+
+ // For testing.
+ void SetUniformBindings(
+ absl::flat_hash_map<std::string, GLint> uniform_locations) {
+ uniform_locations_ = std::move(uniform_locations);
+ }
};
} // namespace impeller
diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc b/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
new file mode 100644
index 0000000..1f7721c
--- /dev/null
+++ b/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
@@ -0,0 +1,50 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/testing/testing.h" // IWYU pragma: keep
+#include "gtest/gtest.h"
+#include "impeller/renderer/backend/gles/buffer_bindings_gles.h"
+#include "impeller/renderer/backend/gles/device_buffer_gles.h"
+#include "impeller/renderer/backend/gles/test/mock_gles.h"
+#include "impeller/renderer/testing/mocks.h"
+
+namespace impeller {
+namespace testing {
+
+TEST(BufferBindingsGLESTest, BindUniformData) {
+ BufferBindingsGLES bindings;
+ absl::flat_hash_map<std::string, GLint> uniform_bindings;
+ uniform_bindings["SHADERMETADATA.FOOBAR"] = 1;
+ bindings.SetUniformBindings(std::move(uniform_bindings));
+ std::shared_ptr<MockGLES> mock_gl = MockGLES::Init();
+ Bindings vertex_bindings;
+
+ ShaderMetadata shader_metadata = {
+ .name = "shader_metadata",
+ .members = {ShaderStructMemberMetadata{.type = ShaderType::kFloat,
+ .name = "foobar",
+ .offset = 0,
+ .size = sizeof(float),
+ .byte_length = sizeof(float)}}};
+ std::shared_ptr<ReactorGLES> reactor;
+ std::shared_ptr<Allocation> backing_store = std::make_shared<Allocation>();
+ ASSERT_TRUE(backing_store->Truncate(Bytes{sizeof(float)}));
+ DeviceBufferGLES device_buffer(DeviceBufferDescriptor{.size = sizeof(float)},
+ reactor, backing_store);
+ BufferView buffer_view(&device_buffer, Range(0, sizeof(float)));
+ vertex_bindings.buffers.push_back(BufferAndUniformSlot{
+ .slot =
+ ShaderUniformSlot{
+ .name = "foobar", .ext_res_0 = 0, .set = 0, .binding = 0},
+ .view = BufferResource(&shader_metadata, buffer_view)});
+ Bindings fragment_bindings;
+ EXPECT_TRUE(bindings.BindUniformData(mock_gl->GetProcTable(), vertex_bindings,
+ fragment_bindings));
+ std::vector<std::string> captured_calls = mock_gl->GetCapturedCalls();
+ EXPECT_TRUE(std::find(captured_calls.begin(), captured_calls.end(),
+ "glUniform1fv") != captured_calls.end());
+}
+
+} // namespace testing
+} // namespace impeller
diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc
index 594fcfa..c172b29 100644
--- a/impeller/renderer/backend/gles/render_pass_gles.cc
+++ b/impeller/renderer/backend/gles/render_pass_gles.cc
@@ -435,7 +435,6 @@
/// Bind uniform data.
///
if (!vertex_desc_gles->BindUniformData(gl, //
- *transients_allocator, //
command.vertex_bindings, //
command.fragment_bindings //
)) {
diff --git a/impeller/renderer/backend/gles/test/mock_gles.cc b/impeller/renderer/backend/gles/test/mock_gles.cc
index 7a8399c..e94e74d 100644
--- a/impeller/renderer/backend/gles/test/mock_gles.cc
+++ b/impeller/renderer/backend/gles/test/mock_gles.cc
@@ -167,6 +167,13 @@
static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT), //
decltype(glDeleteQueriesEXT)>::value);
+void mockUniform1fv(GLint location, GLsizei count, const GLfloat* value) {
+ RecordGLCall("glUniform1fv");
+}
+
+static_assert(CheckSameSignature<decltype(mockUniform1fv), //
+ decltype(glUniform1fv)>::value);
+
std::shared_ptr<MockGLES> MockGLES::Init(
const std::optional<std::vector<const unsigned char*>>& extensions,
const char* version_string,
@@ -208,6 +215,8 @@
return reinterpret_cast<void*>(mockGetQueryObjectui64vEXT);
} else if (strcmp(name, "glGetQueryObjectuivEXT") == 0) {
return reinterpret_cast<void*>(mockGetQueryObjectuivEXT);
+ } else if (strcmp(name, "glUniform1fv") == 0) {
+ return reinterpret_cast<void*>(mockUniform1fv);
} else {
return reinterpret_cast<void*>(&doNothing);
}
