Skip to content

Commit eb9ff8e

Browse files
Anton BikineevSkCQ
Anton Bikineev
authored and
SkCQ
committed
Use 50% load factor for fDigestForPackedGlyphID table
In certain workloads (e.g. Speedometer3, Editor-TipTap) the hash-table lookup becomes very expensive and may require up to 25 probes (specifically for `fDigestForPackedGlyphID`). From local experiments I see that this is not the problem of the hash function, but rather the load faсtor of the HashTable, which is currently hardcoded to 75%. The CL provides a way to customize this behaviour by adding an optional `ShouldGrow(size, capacity)` function into `Traits`. The function is also specialized for `fDigestForPackedGlyphID` to have the load factor of 50%. This significantly reduces the number of extra probes, increasing the ratio of 0-probe hits from 88% to 97% on the overall Speedometer3 benchmark. No more than 5 probes are required. The CL improves the Editor-TipTap Speedometer3 story by 1.3%. Change-Id: Iedda0710839448e8df98d649456a168104488272 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/939016 Commit-Queue: Anton Bikineev <[email protected]> Reviewed-by: Michael Ludwig <[email protected]> Reviewed-by: Ben Wagner <[email protected]>
1 parent 9d51bc2 commit eb9ff8e

File tree

2 files changed

+59
-4
lines changed

2 files changed

+59
-4
lines changed

src/core/SkGlyph.h

+9
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,15 @@ class SkGlyphDigest {
372372
static uint32_t Hash(SkPackedGlyphID packedID) {
373373
return packedID.hash();
374374
}
375+
static bool ShouldGrow(int count, int capacity) {
376+
// Having the 50% load factor results in performance improvements and significantly reduces
377+
// the average number of probes on the Speedometer3 Editor-TipTap benchmark.
378+
return 2 * count >= capacity;
379+
}
380+
static bool ShouldShrink(int count, int capacity) {
381+
// Use 1/6 as the minimal load.
382+
return 6 * count <= capacity;
383+
}
375384

376385
private:
377386
void setAction(skglyph::ActionType actionType, skglyph::GlyphAction action) {

src/core/SkTHash.h

+50-4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ namespace skia_private {
2727
// Traits must have:
2828
// - static K GetKey(T)
2929
// - static uint32_t Hash(K)
30+
// Traits may also define (both required if either is defined):
31+
// - static bool ShouldGrow(int count, int capacity)
32+
// - static bool ShouldShrink(int count, int capacity)
33+
// , which specify the max/min load factor of the table.
3034
// If the key is large and stored inside T, you may want to make K a const&.
3135
// Similarly, if T is large you might want it to be a pointer.
3236
template <typename T, typename K, typename Traits = T>
@@ -98,7 +102,13 @@ class THashTable {
98102
// Copy val into the hash table, returning a pointer to the copy now in the table.
99103
// If there already is an entry in the table with the same key, we overwrite it.
100104
T* set(T val) {
101-
if (4 * fCount >= 3 * fCapacity) {
105+
bool shouldGrow = false;
106+
if constexpr (HasShouldGrow<Traits>::value) {
107+
shouldGrow = Traits::ShouldGrow(fCount, fCapacity);
108+
} else {
109+
shouldGrow = (4 * fCount >= 3 * fCapacity);
110+
}
111+
if (shouldGrow) {
102112
this->resize(fCapacity > 0 ? fCapacity * 2 : 4);
103113
}
104114
return this->uncheckedSet(std::move(val));
@@ -143,8 +153,16 @@ class THashTable {
143153
}
144154
if (hash == s.fHash && key == Traits::GetKey(*s)) {
145155
this->removeSlot(index);
146-
if (4 * fCount <= fCapacity && fCapacity > 4) {
147-
this->resize(fCapacity / 2);
156+
if (fCapacity > 4) {
157+
bool shouldShrink = false;
158+
if constexpr (HasShouldShrink<Traits>::value) {
159+
shouldShrink = Traits::ShouldShrink(fCount, fCapacity);
160+
} else {
161+
shouldShrink = (4 * fCount <= fCapacity);
162+
}
163+
if (shouldShrink) {
164+
this->resize(fCapacity / 2);
165+
}
148166
}
149167
return true;
150168
}
@@ -192,7 +210,14 @@ class THashTable {
192210
// - Hash tables grow when they exceed 3/4 capacity, not when they are full.
193211
void reserve(int n) {
194212
int newCapacity = SkNextPow2(n);
195-
if (n * 4 > newCapacity * 3) {
213+
214+
bool shouldGrow = false;
215+
if constexpr (HasShouldGrow<Traits>::value) {
216+
shouldGrow = Traits::ShouldGrow(n, newCapacity);
217+
} else {
218+
shouldGrow = (n * 4 > newCapacity * 3);
219+
}
220+
if (shouldGrow) {
196221
newCapacity *= 2;
197222
}
198223

@@ -274,6 +299,27 @@ class THashTable {
274299
};
275300

276301
private:
302+
template <typename U, typename = void> struct HasShouldGrow : std::false_type {};
303+
template <typename U, typename = void> struct HasShouldShrink : std::false_type {};
304+
305+
template <typename U>
306+
struct HasShouldGrow<
307+
U,
308+
std::void_t<decltype(U::ShouldGrow(std::declval<int>(), std::declval<int>()))>>
309+
: std::true_type {
310+
static_assert(HasShouldShrink<U>::value,
311+
"The traits class must also provide ShouldShrink() method.");
312+
};
313+
314+
template <typename U>
315+
struct HasShouldShrink<
316+
U,
317+
std::void_t<decltype(U::ShouldShrink(std::declval<int>(), std::declval<int>()))>>
318+
: std::true_type {
319+
static_assert(HasShouldGrow<U>::value,
320+
"The traits class must also provide ShouldGrow() method.");
321+
};
322+
277323
// Finds the first non-empty slot for an iterator.
278324
int firstPopulatedSlot() const {
279325
for (int i = 0; i < fCapacity; i++) {

0 commit comments

Comments
 (0)