Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 96eca99

Browse files
gaaclarkegaaclarke
authored
[impeller] makes UniformBindData 15% faster and adds unit test (#56844)
issue: flutter/flutter#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/Chat
1 parent d727be8 commit 96eca99

File tree

7 files changed

+79
-12
lines changed

7 files changed

+79
-12
lines changed

‎ci/licenses_golden/excluded_files‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@
182182
../../../flutter/impeller/geometry/trig_unittests.cc
183183
../../../flutter/impeller/golden_tests/README.md
184184
../../../flutter/impeller/playground
185+
../../../flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
185186
../../../flutter/impeller/renderer/backend/gles/test
186187
../../../flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm
187188
../../../flutter/impeller/renderer/backend/metal/texture_mtl_unittests.mm

‎impeller/renderer/backend/gles/BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ config("gles_config") {
1414
impeller_component("gles_unittests") {
1515
testonly = true
1616
sources = [
17+
"buffer_bindings_gles_unittests.cc",
1718
"test/capabilities_unittests.cc",
1819
"test/formats_gles_unittests.cc",
1920
"test/gpu_tracer_gles_unittests.cc",

‎impeller/renderer/backend/gles/buffer_bindings_gles.cc‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,16 +180,15 @@ bool BufferBindingsGLES::BindVertexAttributes(const ProcTableGLES& gl,
180180
}
181181

182182
bool BufferBindingsGLES::BindUniformData(const ProcTableGLES& gl,
183-
Allocator& transients_allocator,
184183
const Bindings& vertex_bindings,
185184
const Bindings& fragment_bindings) {
186185
for (const auto& buffer : vertex_bindings.buffers) {
187-
if (!BindUniformBuffer(gl, transients_allocator, buffer.view)) {
186+
if (!BindUniformBuffer(gl, buffer.view)) {
188187
return false;
189188
}
190189
}
191190
for (const auto& buffer : fragment_bindings.buffers) {
192-
if (!BindUniformBuffer(gl, transients_allocator, buffer.view)) {
191+
if (!BindUniformBuffer(gl, buffer.view)) {
193192
return false;
194193
}
195194
}
@@ -262,7 +261,7 @@ const std::vector<GLint>& BufferBindingsGLES::ComputeUniformLocations(
262261
size_t element_count = member.array_elements.value_or(1);
263262
const std::string member_key =
264263
CreateUniformMemberKey(metadata->name, member.name, element_count > 1);
265-
const std::unordered_map<std::string, GLint>::iterator computed_location =
264+
const absl::flat_hash_map<std::string, GLint>::iterator computed_location =
266265
uniform_locations_.find(member_key);
267266
if (computed_location == uniform_locations_.end()) {
268267
// Uniform was not active.
@@ -275,7 +274,6 @@ const std::vector<GLint>& BufferBindingsGLES::ComputeUniformLocations(
275274
}
276275

277276
bool BufferBindingsGLES::BindUniformBuffer(const ProcTableGLES& gl,
278-
Allocator& transients_allocator,
279277
const BufferResource& buffer) {
280278
const ShaderMetadata* metadata = buffer.GetMetadata();
281279
const DeviceBuffer* device_buffer = buffer.resource.GetBuffer();

‎impeller/renderer/backend/gles/buffer_bindings_gles.h‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,18 @@
88
#include <unordered_map>
99
#include <vector>
1010

11+
#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h"
1112
#include "impeller/core/shader_types.h"
1213
#include "impeller/renderer/backend/gles/gles.h"
1314
#include "impeller/renderer/backend/gles/proc_table_gles.h"
1415
#include "impeller/renderer/command.h"
1516

1617
namespace impeller {
1718

19+
namespace testing {
20+
FML_TEST_CLASS(BufferBindingsGLESTest, BindUniformData);
21+
} // namespace testing
22+
1823
//------------------------------------------------------------------------------
1924
/// @brief Sets up stage bindings for single draw call in the OpenGLES
2025
/// backend.
@@ -37,13 +42,13 @@ class BufferBindingsGLES {
3742
size_t vertex_offset);
3843

3944
bool BindUniformData(const ProcTableGLES& gl,
40-
Allocator& transients_allocator,
4145
const Bindings& vertex_bindings,
4246
const Bindings& fragment_bindings);
4347

4448
bool UnbindVertexAttributes(const ProcTableGLES& gl);
4549

4650
private:
51+
FML_FRIEND_TEST(testing::BufferBindingsGLESTest, BindUniformData);
4752
//----------------------------------------------------------------------------
4853
/// @brief The arguments to glVertexAttribPointer.
4954
///
@@ -57,9 +62,9 @@ class BufferBindingsGLES {
5762
};
5863
std::vector<std::vector<VertexAttribPointer>> vertex_attrib_arrays_;
5964

60-
std::unordered_map<std::string, GLint> uniform_locations_;
65+
absl::flat_hash_map<std::string, GLint> uniform_locations_;
6166

62-
using BindingMap = std::unordered_map<std::string, std::vector<GLint>>;
67+
using BindingMap = absl::flat_hash_map<std::string, std::vector<GLint>>;
6368
BindingMap binding_map_ = {};
6469
GLuint vertex_array_object_ = 0;
6570

@@ -68,9 +73,7 @@ class BufferBindingsGLES {
6873

6974
GLint ComputeTextureLocation(const ShaderMetadata* metadata);
7075

71-
bool BindUniformBuffer(const ProcTableGLES& gl,
72-
Allocator& transients_allocator,
73-
const BufferResource& buffer);
76+
bool BindUniformBuffer(const ProcTableGLES& gl, const BufferResource& buffer);
7477

7578
std::optional<size_t> BindTextures(const ProcTableGLES& gl,
7679
const Bindings& bindings,
@@ -80,6 +83,12 @@ class BufferBindingsGLES {
8083
BufferBindingsGLES(const BufferBindingsGLES&) = delete;
8184

8285
BufferBindingsGLES& operator=(const BufferBindingsGLES&) = delete;
86+
87+
// For testing.
88+
void SetUniformBindings(
89+
absl::flat_hash_map<std::string, GLint> uniform_locations) {
90+
uniform_locations_ = std::move(uniform_locations);
91+
}
8392
};
8493

8594
} // namespace impeller

‎impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/testing/testing.h" // IWYU pragma: keep
6+
#include "gtest/gtest.h"
7+
#include "impeller/renderer/backend/gles/buffer_bindings_gles.h"
8+
#include "impeller/renderer/backend/gles/device_buffer_gles.h"
9+
#include "impeller/renderer/backend/gles/test/mock_gles.h"
10+
#include "impeller/renderer/testing/mocks.h"
11+
12+
namespace impeller {
13+
namespace testing {
14+
15+
TEST(BufferBindingsGLESTest, BindUniformData) {
16+
BufferBindingsGLES bindings;
17+
absl::flat_hash_map<std::string, GLint> uniform_bindings;
18+
uniform_bindings["SHADERMETADATA.FOOBAR"] = 1;
19+
bindings.SetUniformBindings(std::move(uniform_bindings));
20+
std::shared_ptr<MockGLES> mock_gl = MockGLES::Init();
21+
Bindings vertex_bindings;
22+
23+
ShaderMetadata shader_metadata = {
24+
.name = "shader_metadata",
25+
.members = {ShaderStructMemberMetadata{.type = ShaderType::kFloat,
26+
.name = "foobar",
27+
.offset = 0,
28+
.size = sizeof(float),
29+
.byte_length = sizeof(float)}}};
30+
std::shared_ptr<ReactorGLES> reactor;
31+
std::shared_ptr<Allocation> backing_store = std::make_shared<Allocation>();
32+
ASSERT_TRUE(backing_store->Truncate(Bytes{sizeof(float)}));
33+
DeviceBufferGLES device_buffer(DeviceBufferDescriptor{.size = sizeof(float)},
34+
reactor, backing_store);
35+
BufferView buffer_view(&device_buffer, Range(0, sizeof(float)));
36+
vertex_bindings.buffers.push_back(BufferAndUniformSlot{
37+
.slot =
38+
ShaderUniformSlot{
39+
.name = "foobar", .ext_res_0 = 0, .set = 0, .binding = 0},
40+
.view = BufferResource(&shader_metadata, buffer_view)});
41+
Bindings fragment_bindings;
42+
EXPECT_TRUE(bindings.BindUniformData(mock_gl->GetProcTable(), vertex_bindings,
43+
fragment_bindings));
44+
std::vector<std::string> captured_calls = mock_gl->GetCapturedCalls();
45+
EXPECT_TRUE(std::find(captured_calls.begin(), captured_calls.end(),
46+
"glUniform1fv") != captured_calls.end());
47+
}
48+
49+
} // namespace testing
50+
} // namespace impeller

‎impeller/renderer/backend/gles/render_pass_gles.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
435435
/// Bind uniform data.
436436
///
437437
if (!vertex_desc_gles->BindUniformData(gl, //
438-
*transients_allocator, //
439438
command.vertex_bindings, //
440439
command.fragment_bindings //
441440
)) {

‎impeller/renderer/backend/gles/test/mock_gles.cc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,13 @@ void mockDeleteTextures(GLsizei size, const GLuint* queries) {
167167
static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT), //
168168
decltype(glDeleteQueriesEXT)>::value);
169169

170+
void mockUniform1fv(GLint location, GLsizei count, const GLfloat* value) {
171+
RecordGLCall("glUniform1fv");
172+
}
173+
174+
static_assert(CheckSameSignature<decltype(mockUniform1fv), //
175+
decltype(glUniform1fv)>::value);
176+
170177
std::shared_ptr<MockGLES> MockGLES::Init(
171178
const std::optional<std::vector<const unsigned char*>>& extensions,
172179
const char* version_string,
@@ -208,6 +215,8 @@ const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) {
208215
return reinterpret_cast<void*>(mockGetQueryObjectui64vEXT);
209216
} else if (strcmp(name, "glGetQueryObjectuivEXT") == 0) {
210217
return reinterpret_cast<void*>(mockGetQueryObjectuivEXT);
218+
} else if (strcmp(name, "glUniform1fv") == 0) {
219+
return reinterpret_cast<void*>(mockUniform1fv);
211220
} else {
212221
return reinterpret_cast<void*>(&doNothing);
213222
}

0 commit comments

Comments
 (0)