Skip to content

Commit 6070afc

Browse files
iamirzhanmeta-codesync[bot]
authored andcommitted
Cache uset merges
Summary: I did some profiling on `glean complete` for fbsource db which takes 6 hours: * The last `completeOwnership` step takes 80% of total time. * In the `completeOwnership` function 83% of time is taken by merging usets {F1984035266} So, I am adding a simple approximate LRU cache with 2 maps (old/new) for merge operations. I've tried a proper LRU cache first, but it creates additional performance overhead comparing to simple cache. The cache hit ratio is high in both cases. The current implementation is already caching some results, but only per fact and clearing everything on each iteration. With a separate cache the cache hit ratio increases from 70% to 88%. The total performance improvement is 15%. Next I will try to parallelise merging step Reviewed By: malanka Differential Revision: D88745264 fbshipit-source-id: ff2f5225fabffe997066612ea6b15313c4869ff9
1 parent 6f7c3c3 commit 6070afc

1 file changed

Lines changed: 116 additions & 32 deletions

File tree

glean/rts/ownership.cpp

Lines changed: 116 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
#include "glean/rts/ownership.h"
10-
#include "glean/rts/factset.h"
1110
#include "glean/rts/inventory.h"
1211
#include "glean/rts/lookup.h"
1312
#include "glean/rts/ownership/setu32.h"
@@ -29,8 +28,6 @@
2928

3029
#include <algorithm>
3130
#include <initializer_list>
32-
#include <limits>
33-
#include <type_traits>
3431

3532
namespace facebook {
3633
namespace glean {
@@ -210,6 +207,8 @@ FOLLY_NOINLINE void completeOwnership(
210207
size_t local_facts = 0;
211208
size_t base_facts = 0;
212209
size_t owned_facts = 0;
210+
size_t cache_hits = 0;
211+
size_t cache_misses = 0;
213212

214213
void bumpLocal() {
215214
++local_facts;
@@ -226,7 +225,25 @@ FOLLY_NOINLINE void completeOwnership(
226225
}
227226
}
228227

228+
void bumpCacheHit() {
229+
++cache_hits;
230+
}
231+
232+
void bumpCacheMiss() {
233+
++cache_misses;
234+
}
235+
236+
double hitRate() const {
237+
size_t total = cache_hits + cache_misses;
238+
return total > 0 ? (cache_hits * 100.0 / total) : 0.0;
239+
}
240+
229241
void dump() {
242+
VLOG(1) << folly::sformat(
243+
"SetUnionsCache: {} hits, {} misses, hit rate: {:.2f}%",
244+
cache_hits,
245+
cache_misses,
246+
hitRate());
230247
auto ustats = usets.statistics();
231248
VLOG(1) << folly::sformat(
232249
"{} of {} facts ({} visited in base DBs), {} usets, {} promoted, {} bytes, {} adds, {} dups",
@@ -241,7 +258,86 @@ FOLLY_NOINLINE void completeOwnership(
241258
}
242259
};
243260

261+
constexpr size_t SET_UNIONS_CACHE_SIZE = 100;
262+
struct UsetUnionsCacheKey {
263+
Uset* a;
264+
Uset* b;
265+
266+
UsetUnionsCacheKey(Uset* x, Uset* y) {
267+
// Normalize as order doesn't matter. (A∪B = B∪A)
268+
a = (x < y) ? x : y;
269+
b = (x < y) ? y : x;
270+
}
271+
272+
bool operator==(const UsetUnionsCacheKey& o) const {
273+
return (a == o.a && b == o.b);
274+
}
275+
276+
size_t hash() const {
277+
return folly::hash::hash_combine(a->hash, b->hash);
278+
}
279+
};
280+
281+
struct UsetUnionsCacheKeyHash {
282+
size_t operator()(const UsetUnionsCacheKey& k) const {
283+
return k.hash();
284+
}
285+
};
286+
287+
class UsetUnionsCache {
288+
using Map =
289+
folly::F14FastMap<UsetUnionsCacheKey, Uset*, UsetUnionsCacheKeyHash>;
290+
291+
Map new_cache;
292+
Map old_cache;
293+
Usets& usets;
294+
size_t max_size;
295+
296+
void evictOldCache() {
297+
for (auto& [key, value] : old_cache) {
298+
usets.drop(key.a);
299+
usets.drop(key.b);
300+
usets.drop(value);
301+
}
302+
old_cache.clear();
303+
}
304+
305+
public:
306+
UsetUnionsCache(size_t max_size, Usets& usets)
307+
: usets(usets), max_size(max_size) {}
308+
309+
Uset* find(const UsetUnionsCacheKey& key) {
310+
auto it = new_cache.find(key);
311+
if (it != new_cache.end()) {
312+
return it->second;
313+
}
314+
it = old_cache.find(key);
315+
if (it != old_cache.end()) {
316+
return it->second;
317+
}
318+
return nullptr;
319+
}
320+
321+
void insert(const UsetUnionsCacheKey& key, Uset* value) {
322+
if (new_cache.size() >= max_size) {
323+
evictOldCache();
324+
std::swap(old_cache, new_cache);
325+
}
326+
new_cache.insert({key, value});
327+
usets.use(key.a);
328+
usets.use(key.b);
329+
usets.use(value);
330+
}
331+
332+
void clear() {
333+
evictOldCache();
334+
std::swap(old_cache, new_cache);
335+
evictOldCache();
336+
}
337+
};
338+
244339
Stats stats{usets};
340+
UsetUnionsCache cache(SET_UNIONS_CACHE_SIZE, usets);
245341

246342
std::vector<Id> refs;
247343
const auto tracker = syscall([&refs](Id id, Pid) { refs.push_back(id); });
@@ -283,45 +379,32 @@ FOLLY_NOINLINE void completeOwnership(
283379
assert(predicate);
284380
predicate->traverse(tracker, fact.clause);
285381

286-
// For each fact we reference, add our ownership set to what we've
287-
// computed for it so far. Use the `link` field to avoid computing the
288-
// same set union multiple times.
289-
//
290-
// TODO: Try adding a fixed-size LRU (or LFU?) cache for set unions?
291-
std::vector<Uset*> touched;
292382
for (const auto id : refs) {
293383
auto& me = owner(id);
294384
if (me == nullptr) {
295385
// The fact didn't have ownership info before, assign the set to it.
296386
me = set;
297387
usets.use(me, 1);
298-
} else if (const auto added = static_cast<Uset*>(me->link())) {
299-
// The fact did have ownership info and we've already computed the
300-
// union for that particular set.
301-
usets.use(added);
302-
usets.drop(me);
303-
me = added;
304388
} else {
305-
// Compute the union.
306-
const auto p = usets.merge(me, set);
307-
me->link(p);
308-
touched.push_back(me);
309-
me = p;
389+
UsetUnionsCacheKey key(me, set);
390+
auto* cached = cache.find(key);
391+
if (cached != nullptr) {
392+
stats.bumpCacheHit();
393+
usets.use(cached);
394+
usets.drop(me);
395+
me = cached;
396+
} else {
397+
stats.bumpCacheMiss();
398+
const auto p = usets.merge(me, set);
399+
cache.insert(key, p);
400+
usets.drop(me);
401+
me = p;
402+
}
310403
}
311404
}
312405

313-
// Reset the link fields. Note that we always add 1 refcount for sets we
314-
// store in `touched` (to avoid having dangling references there) so drop
315-
// it here.
316-
for (const auto p : touched) {
317-
p->link(nullptr);
318-
usets.drop(p);
319-
}
320-
321-
touched.clear();
322-
// TODO: We can drop the current's fact ownership info here - could shrink
323-
// the vector.
324-
// facts.shrink(fact.id);
406+
// TODO: We can drop the current's fact ownership info here - could
407+
// shrink the vector. facts.shrink(fact.id);
325408
stats.bumpOwned();
326409
}
327410
};
@@ -411,6 +494,7 @@ FOLLY_NOINLINE void completeOwnership(
411494
}
412495

413496
stats.dump();
497+
cache.clear();
414498
}
415499
} // namespace
416500

0 commit comments

Comments
 (0)