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

Commit 516235e

Browse files
chinmaygardechinmaygarde
authored
[Impeller] Use the typed allocation sizes in impeller::Allocation. (#54327)
1 parent dda6c81 commit 516235e

File tree

11 files changed

+161
-47
lines changed

11 files changed

+161
-47
lines changed

‎impeller/base/allocation.cc

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ uint8_t* Allocation::GetBuffer() const {
2121
return buffer_;
2222
}
2323

24-
size_t Allocation::GetLength() const {
24+
Bytes Allocation::GetLength() const {
2525
return length_;
2626
}
2727

28-
size_t Allocation::GetReservedLength() const {
28+
Bytes Allocation::GetReservedLength() const {
2929
return reserved_;
3030
}
3131

32-
bool Allocation::Truncate(size_t length, bool npot) {
32+
bool Allocation::Truncate(Bytes length, bool npot) {
3333
const auto reserved = npot ? ReserveNPOT(length) : Reserve(length);
3434
if (!reserved) {
3535
return false;
@@ -54,18 +54,18 @@ uint32_t Allocation::NextPowerOfTwoSize(uint32_t x) {
5454
return x + 1;
5555
}
5656

57-
bool Allocation::ReserveNPOT(size_t reserved) {
57+
bool Allocation::ReserveNPOT(Bytes reserved) {
5858
// Reserve at least one page of data.
59-
reserved = std::max<size_t>(4096u, reserved);
60-
return Reserve(NextPowerOfTwoSize(reserved));
59+
reserved = std::max(Bytes{4096u}, reserved);
60+
return Reserve(Bytes{NextPowerOfTwoSize(reserved.GetByteSize())});
6161
}
6262

63-
bool Allocation::Reserve(size_t reserved) {
63+
bool Allocation::Reserve(Bytes reserved) {
6464
if (reserved <= reserved_) {
6565
return true;
6666
}
6767

68-
auto new_allocation = ::realloc(buffer_, reserved);
68+
auto new_allocation = ::realloc(buffer_, reserved.GetByteSize());
6969
if (!new_allocation) {
7070
// If new length is zero, a minimum non-zero sized allocation is returned.
7171
// So this check will not trip and this routine will indicate success as
@@ -81,7 +81,7 @@ bool Allocation::Reserve(size_t reserved) {
8181
}
8282

8383
std::shared_ptr<fml::Mapping> CreateMappingWithCopy(const uint8_t* contents,
84-
size_t length) {
84+
Bytes length) {
8585
if (contents == nullptr) {
8686
return nullptr;
8787
}
@@ -91,7 +91,7 @@ std::shared_ptr<fml::Mapping> CreateMappingWithCopy(const uint8_t* contents,
9191
return nullptr;
9292
}
9393

94-
std::memmove(allocation->GetBuffer(), contents, length);
94+
std::memmove(allocation->GetBuffer(), contents, length.GetByteSize());
9595

9696
return CreateMappingFromAllocation(allocation);
9797
}
@@ -103,7 +103,7 @@ std::shared_ptr<fml::Mapping> CreateMappingFromAllocation(
103103
}
104104
return std::make_shared<fml::NonOwnedMapping>(
105105
reinterpret_cast<const uint8_t*>(allocation->GetBuffer()), //
106-
allocation->GetLength(), //
106+
allocation->GetLength().GetByteSize(), //
107107
[allocation](auto, auto) {} //
108108
);
109109
}

‎impeller/base/allocation.h

Lines changed: 105 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,45 +13,138 @@
1313

1414
namespace impeller {
1515

16+
//------------------------------------------------------------------------------
17+
/// @brief Describes an allocation on the heap.
18+
///
19+
/// Managing allocations through this utility makes it harder to
20+
/// miss allocation failures.
21+
///
1622
class Allocation {
1723
public:
24+
//----------------------------------------------------------------------------
25+
/// @brief Constructs a new zero-sized allocation.
26+
///
1827
Allocation();
1928

29+
//----------------------------------------------------------------------------
30+
/// @brief Destroys the allocation.
31+
///
2032
~Allocation();
2133

34+
//----------------------------------------------------------------------------
35+
/// @brief Gets the pointer to the start of the allocation.
36+
///
37+
/// This pointer is only valid till the next call to `Truncate`.
38+
///
39+
/// @return The pointer to the start of the allocation.
40+
///
2241
uint8_t* GetBuffer() const;
2342

24-
size_t GetLength() const;
25-
26-
size_t GetReservedLength() const;
27-
28-
[[nodiscard]] bool Truncate(size_t length, bool npot = true);
29-
43+
//----------------------------------------------------------------------------
44+
/// @brief Gets the length of the allocation.
45+
///
46+
/// @return The length.
47+
///
48+
Bytes GetLength() const;
49+
50+
//----------------------------------------------------------------------------
51+
/// @brief Gets the reserved length of the allocation. Calls to truncate
52+
/// may be ignored till the length exceeds the reserved length.
53+
///
54+
/// @return The reserved length.
55+
///
56+
Bytes GetReservedLength() const;
57+
58+
//----------------------------------------------------------------------------
59+
/// @brief Resize the underlying allocation to at least given number of
60+
/// bytes.
61+
///
62+
/// In case of failure, false is returned and the underlying
63+
/// allocation remains unchanged.
64+
///
65+
/// @warning Pointers to buffers obtained via previous calls to `GetBuffer`
66+
/// may become invalid at this point.
67+
///
68+
/// @param[in] length The length.
69+
/// @param[in] npot Whether to round up the length to the next power of
70+
/// two.
71+
///
72+
/// @return If the underlying allocation was resized to the new size.
73+
///
74+
[[nodiscard]] bool Truncate(Bytes length, bool npot = true);
75+
76+
//----------------------------------------------------------------------------
77+
/// @brief Gets the next power of two size.
78+
///
79+
/// @param[in] x The size.
80+
///
81+
/// @return The next power of two of x.
82+
///
3083
static uint32_t NextPowerOfTwoSize(uint32_t x);
3184

3285
private:
3386
uint8_t* buffer_ = nullptr;
34-
size_t length_ = 0;
35-
size_t reserved_ = 0;
87+
Bytes length_;
88+
Bytes reserved_;
3689

37-
[[nodiscard]] bool Reserve(size_t reserved);
90+
[[nodiscard]] bool Reserve(Bytes reserved);
3891

39-
[[nodiscard]] bool ReserveNPOT(size_t reserved);
92+
[[nodiscard]] bool ReserveNPOT(Bytes reserved);
4093

4194
Allocation(const Allocation&) = delete;
4295

4396
Allocation& operator=(const Allocation&) = delete;
4497
};
4598

99+
//------------------------------------------------------------------------------
100+
/// @brief Creates a mapping with copy of the bytes.
101+
///
102+
/// @param[in] contents The contents
103+
/// @param[in] length The length
104+
///
105+
/// @return The new mapping or nullptr if the copy could not be performed.
106+
///
46107
std::shared_ptr<fml::Mapping> CreateMappingWithCopy(const uint8_t* contents,
47-
size_t length);
48-
108+
Bytes length);
109+
110+
//------------------------------------------------------------------------------
111+
/// @brief Creates a mapping from allocation.
112+
///
113+
/// No data copy occurs. Only a reference to the underlying
114+
/// allocation is bumped up.
115+
///
116+
/// Changes to the underlying allocation will not be reflected in
117+
/// the mapping and must not change.
118+
///
119+
/// @param[in] allocation The allocation.
120+
///
121+
/// @return A new mapping or nullptr if the argument allocation was invalid.
122+
///
49123
std::shared_ptr<fml::Mapping> CreateMappingFromAllocation(
50124
const std::shared_ptr<Allocation>& allocation);
51125

126+
//------------------------------------------------------------------------------
127+
/// @brief Creates a mapping with string data.
128+
///
129+
/// Only a reference to the underlying string is bumped up and the
130+
/// string is not copied.
131+
///
132+
/// @param[in] string The string
133+
///
134+
/// @return A new mapping or nullptr in case of allocation failures.
135+
///
52136
std::shared_ptr<fml::Mapping> CreateMappingWithString(
53137
std::shared_ptr<const std::string> string);
54138

139+
//------------------------------------------------------------------------------
140+
/// @brief Creates a mapping with string data.
141+
///
142+
/// The string is copied.
143+
///
144+
/// @param[in] string The string
145+
///
146+
/// @return A new mapping or nullptr in case of allocation failures.
147+
///
55148
std::shared_ptr<fml::Mapping> CreateMappingWithString(std::string string);
56149

57150
} // namespace impeller

‎impeller/base/allocation_size.h

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
#ifndef FLUTTER_IMPELLER_BASE_ALLOCATION_SIZE_H_
66
#define FLUTTER_IMPELLER_BASE_ALLOCATION_SIZE_H_
77

8+
#include <cmath>
89
#include <cstddef>
910
#include <cstdint>
11+
#include <type_traits>
1012

1113
namespace impeller {
1214

@@ -37,7 +39,9 @@ class AllocationSize {
3739
///
3840
/// @param[in] size The size in `Period` number of bytes.
3941
///
40-
explicit constexpr AllocationSize(double size) : bytes_(size * Period) {}
42+
template <class T, class = std::enable_if_t<std::is_arithmetic_v<T>>>
43+
explicit constexpr AllocationSize(T size)
44+
: bytes_(std::ceil(size) * Period) {}
4145

4246
//----------------------------------------------------------------------------
4347
/// @brief Create an allocation size from another instance with a
@@ -158,37 +162,37 @@ inline namespace allocation_size_literals {
158162

159163
// NOLINTNEXTLINE
160164
constexpr Bytes operator"" _bytes(unsigned long long int size) {
161-
return Bytes{static_cast<double>(size)};
165+
return Bytes{size};
162166
}
163167

164168
// NOLINTNEXTLINE
165169
constexpr KiloBytes operator"" _kb(unsigned long long int size) {
166-
return KiloBytes{static_cast<double>(size)};
170+
return KiloBytes{size};
167171
}
168172

169173
// NOLINTNEXTLINE
170174
constexpr MegaBytes operator"" _mb(unsigned long long int size) {
171-
return MegaBytes{static_cast<double>(size)};
175+
return MegaBytes{size};
172176
}
173177

174178
// NOLINTNEXTLINE
175179
constexpr GigaBytes operator"" _gb(unsigned long long int size) {
176-
return GigaBytes{static_cast<double>(size)};
180+
return GigaBytes{size};
177181
}
178182

179183
// NOLINTNEXTLINE
180184
constexpr KibiBytes operator"" _kib(unsigned long long int size) {
181-
return KibiBytes{static_cast<double>(size)};
185+
return KibiBytes{size};
182186
}
183187

184188
// NOLINTNEXTLINE
185189
constexpr MebiBytes operator"" _mib(unsigned long long int size) {
186-
return MebiBytes{static_cast<double>(size)};
190+
return MebiBytes{size};
187191
}
188192

189193
// NOLINTNEXTLINE
190194
constexpr GibiBytes operator"" _gib(unsigned long long int size) {
191-
return GibiBytes{static_cast<double>(size)};
195+
return GibiBytes{size};
192196
}
193197

194198
} // namespace allocation_size_literals

‎impeller/base/allocation_size_unittests.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,19 @@ TEST(AllocationSizeTest, CanPerformSimpleArithmetic) {
107107
}
108108
}
109109

110+
TEST(AllocationSizeTest, CanConstructWithArith) {
111+
{
112+
Bytes a(1u);
113+
ASSERT_EQ(a.GetByteSize(), 1u);
114+
}
115+
{
116+
Bytes a(1.5);
117+
ASSERT_EQ(a.GetByteSize(), 2u);
118+
}
119+
{
120+
Bytes a(1.5f);
121+
ASSERT_EQ(a.GetByteSize(), 2u);
122+
}
123+
}
124+
110125
} // namespace impeller::testing

‎impeller/playground/image/decompressed_image.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ DecompressedImage DecompressedImage::ConvertToRGBA() const {
7575
}
7676

7777
auto rgba_allocation = std::make_shared<Allocation>();
78-
if (!rgba_allocation->Truncate(size_.Area() * 4u, false)) {
78+
if (!rgba_allocation->Truncate(Bytes{size_.Area() * 4u}, false)) {
7979
return {};
8080
}
8181

@@ -114,9 +114,9 @@ DecompressedImage DecompressedImage::ConvertToRGBA() const {
114114
return DecompressedImage{
115115
size_, Format::kRGBA,
116116
std::make_shared<fml::NonOwnedMapping>(
117-
rgba_allocation->GetBuffer(), //
118-
rgba_allocation->GetLength(), //
119-
[rgba_allocation](auto, auto) {}) //
117+
rgba_allocation->GetBuffer(), //
118+
rgba_allocation->GetLength().GetByteSize(), //
119+
[rgba_allocation](auto, auto) {}) //
120120
};
121121
}
122122

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ bool AllocatorGLES::IsValid() const {
2828
std::shared_ptr<DeviceBuffer> AllocatorGLES::OnCreateBuffer(
2929
const DeviceBufferDescriptor& desc) {
3030
auto backing_store = std::make_shared<Allocation>();
31-
if (!backing_store->Truncate(desc.size)) {
31+
if (!backing_store->Truncate(Bytes{desc.size})) {
3232
return nullptr;
3333
}
3434
return std::make_shared<DeviceBufferGLES>(desc, //

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ bool DeviceBufferGLES::OnCopyHostBuffer(const uint8_t* source,
4646
return false;
4747
}
4848

49-
if (offset + source_range.length > backing_store_->GetLength()) {
49+
if (offset + source_range.length >
50+
backing_store_->GetLength().GetByteSize()) {
5051
return false;
5152
}
5253

@@ -87,9 +88,10 @@ bool DeviceBufferGLES::BindAndUploadDataIfNecessary(BindingType type) const {
8788
gl.BindBuffer(target_type, buffer.value());
8889

8990
if (upload_generation_ != generation_) {
90-
TRACE_EVENT1("impeller", "BufferData", "Bytes",
91-
std::to_string(backing_store_->GetLength()).c_str());
92-
gl.BufferData(target_type, backing_store_->GetLength(),
91+
TRACE_EVENT1(
92+
"impeller", "BufferData", "Bytes",
93+
std::to_string(backing_store_->GetLength().GetByteSize()).c_str());
94+
gl.BufferData(target_type, backing_store_->GetLength().GetByteSize(),
9395
backing_store_->GetBuffer(), GL_STATIC_DRAW);
9496
upload_generation_ = generation_;
9597
}
@@ -119,7 +121,7 @@ void DeviceBufferGLES::UpdateBufferData(
119121
update_buffer_data) {
120122
if (update_buffer_data) {
121123
update_buffer_data(backing_store_->GetBuffer(),
122-
backing_store_->GetLength());
124+
backing_store_->GetLength().GetByteSize());
123125
++generation_;
124126
}
125127
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ std::string ProcTableGLES::GetProgramInfoLogString(GLuint program) const {
407407

408408
length = std::min<GLint>(length, 1024);
409409
Allocation allocation;
410-
if (!allocation.Truncate(length, false)) {
410+
if (!allocation.Truncate(Bytes{length}, false)) {
411411
return "";
412412
}
413413
GetProgramInfoLog(program, // program

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ void TextureGLES::SetLabel(std::string_view label) {
201201
bool TextureGLES::OnSetContents(const uint8_t* contents,
202202
size_t length,
203203
size_t slice) {
204-
return OnSetContents(CreateMappingWithCopy(contents, length), slice);
204+
return OnSetContents(CreateMappingWithCopy(contents, Bytes{length}), slice);
205205
}
206206

207207
// |Texture|

‎impeller/renderer/backend/metal/allocator_mtl.mm

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,7 @@ static bool SupportsLossyTextureCompression(id<MTLDevice> device) {
100100
}
101101

102102
Bytes DebugAllocatorStats::GetAllocationSize() {
103-
// RAM is measured in MiB, thus a divisor of 2^20 instead of 1,000,000.
104-
return Bytes{static_cast<double>(size_)};
103+
return Bytes{size_.load()};
105104
}
106105

107106
AllocatorMTL::AllocatorMTL(id<MTLDevice> device, std::string label)

0 commit comments

Comments
 (0)