AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Clamp block increment to uint16 max instead of asserting

Browse Source 
The block increment parameter, after dividing by address align, has to
fit into 16 bits. SkTBlockList with either large T or a large
"itemsPerBlock" hint can pretty easily exceed this. However, both the
items per block and the block increment are just hints to control
allocation patterns. SkBlockAllocator can have larger blocks than that
based on growth policy, up to its actual allocation size limit.
SkTBlockList also does not need itemsPerBlock in its implementation, so
if the request exceeds what the allocator can do, it's not problematic.

So clamping to the highest storable value is nicer than asserting that
the caller respected the internal limits.

Change-Id: I82b1c20034fd264b65c7eae4d6758caa6b574fb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520656
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Michael Ludwig 2022-03-14 15:21:49 -04:00 committed by SkCQ
parent 965d9dc948
commit fc4fb7a624
2 changed files with 21 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/core/SkBlockAllocator.cpp
View File

@ -16,8 +16,9 @@ SkBlockAllocator::SkBlockAllocator(GrowthPolicy policy, size_t blockIncrementByt
: fTail(&fHead)
// Round up to the nearest max-aligned value, and then divide so that fBlockSizeIncrement
// can effectively fit higher byte counts in its 16 bits of storage
, fBlockIncrement(SkTo<uint16_t>(SkAlignTo(blockIncrementBytes, kAddressAlign)
/ kAddressAlign))
, fBlockIncrement(SkTo<uint16_t>(
std::min(SkAlignTo(blockIncrementBytes, kAddressAlign) / kAddressAlign,
(size_t) std::numeric_limits<uint16_t>::max())))
, fGrowthPolicy(static_cast<uint64_t>(policy))
, fN0((policy == GrowthPolicy::kLinear || policy == GrowthPolicy::kExponential) ? 1 : 0)
, fN1(1)

18
tests/SkTBlockListTest.cpp
View File

@ -48,6 +48,8 @@ public:
static size_t TotalSize(SkTBlockList<C, N>& list) {
return list.fAllocator->totalSize();
}
static constexpr size_t kAddressAlign = SkBlockAllocator::kAddressAlign;
};
// Checks that the allocator has the correct count, etc and that the element IDs are correct.
@ -300,6 +302,20 @@ static void run_reserve_test(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, 0 == C::gInstCnt);
}
void run_large_increment_test(skiatest::Reporter* reporter) {
static constexpr size_t kIncrementMax = std::numeric_limits<uint16_t>::max();
// Pick an item count such that count*sizeof(C)/max align exceeds uint16_t max.
int itemCount = (int) (sizeof(C) * kIncrementMax / TBlockListTestAccess::kAddressAlign) + 1;
SkTBlockList<C> largeIncrement(itemCount);
// Trigger a scratch block allocation, which given default fixed growth policy, will be one
// block increment.
largeIncrement.reserve(10);
size_t scratchSize = TBlockListTestAccess::ScratchBlockSize(largeIncrement);
// SkBlockAllocator aligns large blocks to 4k
size_t expected = SkAlignTo(kIncrementMax * TBlockListTestAccess::kAddressAlign, (1 << 12));
REPORTER_ASSERT(reporter, scratchSize == expected);
}
DEF_TEST(SkTBlockList, reporter) {
// Test combinations of allocators with and without stack storage and with different block sizes
SkTBlockList<C> a1(1);
@ -335,4 +351,6 @@ DEF_TEST(SkTBlockList, reporter) {
run_concat_trivial_test<5, 1>(reporter, 50, 10);
run_concat_trivial_test<1, 5>(reporter, 10, 50);
run_concat_trivial_test<5, 5>(reporter, 100, 100);
run_large_increment_test(reporter);
}