facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-04 13:57:13 +03:00
Code Issues Projects Releases Wiki Activity

avoid calling getStats() in some ObjectCache fb303 callbacks

Browse Source 
Summary: The current state of this code causes an infinite loop if trying to use `getStats` for the `eden stats` cli in D56453268. This diff introduces a specific helper function to extract these singular values. Also, we don't need to collect the whole of the `Stats` objects anyways to feed these two dynamic counters anyways.

Reviewed By: kmancini

Differential Revision: D56728227

fbshipit-source-id: 22523f97a6864464beed875e82fef1d99348fe2a
This commit is contained in:
Genevieve (Genna) Helsel 2024-05-03 10:14:40 -07:00 committed by Facebook GitHub Bot
parent 1b1009edcd
commit 011e8dc1c9
6 changed files with 50 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
eden/fs/store/BlobCache.cpp
View File

@ -72,9 +72,9 @@ BlobCache::~BlobCache() {
void BlobCache::registerStats() {
auto counters = fb303::ServiceData::get()->getDynamicCounters();
counters->registerCallback(
kBlobCacheMemory, [this] { return getStats().totalSizeInBytes; });
kBlobCacheMemory, [this] { return getTotalSizeBytes(); });
counters->registerCallback(
kBlobCacheItems, [this] { return getStats().objectCount; });
kBlobCacheItems, [this] { return getObjectCount(); });
}
} // namespace facebook::eden

20
eden/fs/store/ObjectCache-inl.h
View File

@ -332,6 +332,26 @@ void ObjectCache<ObjectType, Flavor, ObjectCacheStats>::clear() {
state->items.clear();
}
template <
typename ObjectType,
ObjectCacheFlavor Flavor,
typename ObjectCacheStats>
size_t ObjectCache<ObjectType, Flavor, ObjectCacheStats>::getTotalSizeBytes()
const {
auto state = state_.lock();
return state->totalSize;
}
template <
typename ObjectType,
ObjectCacheFlavor Flavor,
typename ObjectCacheStats>
size_t ObjectCache<ObjectType, Flavor, ObjectCacheStats>::getObjectCount()
const {
auto state = state_.lock();
return state->items.size();
}
template <
typename ObjectType,
ObjectCacheFlavor Flavor,

12
eden/fs/store/ObjectCache.h
View File

@ -244,6 +244,18 @@ class ObjectCache : public std::enable_shared_from_this<
*/
void clear();
/**
* Returns the memory footprint of the cache. This is meant to be used for
* dynamic counter registration
*/
size_t getTotalSizeBytes() const;
/**
* Returns the number of objects in the cache. This is meant to be used for
* dynamic counter registration
*/
size_t getObjectCount() const;
/**
* Return information about the current size of the cache and the total number
* of hits and misses.

4
eden/fs/store/TreeCache.cpp
View File

@ -45,9 +45,9 @@ TreeCache::~TreeCache() {
void TreeCache::registerStats() {
auto counters = fb303::ServiceData::get()->getDynamicCounters();
counters->registerCallback(
kTreeCacheMemory, [this] { return getStats().totalSizeInBytes; });
kTreeCacheMemory, [this] { return getTotalSizeBytes(); });
counters->registerCallback(
kTreeCacheItems, [this] { return getStats().objectCount; });
kTreeCacheItems, [this] { return getObjectCount(); });
}
} // namespace facebook::eden

12
eden/fs/store/test/BlobCacheTest.cpp
View File

@ -54,14 +54,14 @@ TEST_F(BlobCacheTest, evicts_oldest_on_insertion) {
auto cache = BlobCache::create(10, 0, edenConfig, makeRefPtr<EdenStats>());
cache->insert(hash3, blob3);
cache->insert(hash4, blob4); // blob4 is considered more recent than blob3
EXPECT_EQ(7, cache->getStats().totalSizeInBytes);
EXPECT_EQ(7, cache->getTotalSizeBytes());
cache->insert(hash5, blob5); // evicts blob3
EXPECT_EQ(9, cache->getStats().totalSizeInBytes);
EXPECT_EQ(9, cache->getTotalSizeBytes());
EXPECT_EQ(nullptr, cache->get(hash3).object)
<< "Inserting blob5 should evict oldest (blob3)";
EXPECT_EQ(blob4, cache->get(hash4).object) << "But blob4 still fits";
cache->insert(hash3, blob3); // evicts blob5
EXPECT_EQ(7, cache->getStats().totalSizeInBytes);
EXPECT_EQ(7, cache->getTotalSizeBytes());
EXPECT_EQ(nullptr, cache->get(hash5).object)
<< "Inserting blob3 again evicts blob5 because blob4 was accessed";
EXPECT_EQ(blob4, cache->get(hash4).object);
@ -84,7 +84,7 @@ TEST_F(BlobCacheTest, preserves_minimum_number_of_entries) {
cache->insert(hash5, blob5);
cache->insert(hash6, blob6);
EXPECT_EQ(15, cache->getStats().totalSizeInBytes);
EXPECT_EQ(15, cache->getTotalSizeBytes());
EXPECT_FALSE(cache->get(hash3).object);
EXPECT_TRUE(cache->get(hash4).object);
EXPECT_TRUE(cache->get(hash5).object);
@ -149,13 +149,13 @@ TEST_F(BlobCacheTest, no_blob_caching) {
cache->insert(hash4, blob4);
cache->insert(hash5, blob5);
// Cache should be empty since it is turned off
EXPECT_EQ(0, cache->getStats().totalSizeInBytes);
EXPECT_EQ(0, cache->getTotalSizeBytes());
auto blob = std::make_shared<Blob>("newblob"_sp);
auto weak = std::weak_ptr<const Blob>{blob};
auto handle = cache->insert(hash6, blob, BlobCache::Interest::WantHandle);
// Cache should be empty since it is turned off
EXPECT_EQ(0, cache->getStats().totalSizeInBytes);
EXPECT_EQ(0, cache->getTotalSizeBytes());
auto handle0 = cache->insert(hash6, blob, BlobCache::Interest::WantHandle);
// Inserting should still return the object

16
eden/fs/store/test/ObjectCacheTest.cpp
View File

@ -239,15 +239,15 @@ TEST(ObjectCache, interest_handle_evicts_oldest_on_insertion) {
cache->insertInterestHandle(
object4->getHash(),
object4); // object4 is considered more recent than object3
EXPECT_EQ(7, cache->getStats().totalSizeInBytes);
EXPECT_EQ(7, cache->getTotalSizeBytes());
cache->insertInterestHandle(object5->getHash(), object5); // evicts object3
EXPECT_EQ(9, cache->getStats().totalSizeInBytes);
EXPECT_EQ(9, cache->getTotalSizeBytes());
EXPECT_EQ(nullptr, cache->getInterestHandle(hash3).object)
<< "Inserting object5 should evict oldest (object3)";
EXPECT_EQ(object4, cache->getInterestHandle(hash4).object)
<< "But object4 still fits";
cache->insertInterestHandle(object3->getHash(), object3); // evicts object5
EXPECT_EQ(7, cache->getStats().totalSizeInBytes);
EXPECT_EQ(7, cache->getTotalSizeBytes());
EXPECT_EQ(nullptr, cache->getInterestHandle(hash5).object)
<< "Inserting object3 again evicts object5 because object4 was accessed";
EXPECT_EQ(object4, cache->getInterestHandle(hash4).object);
@ -293,7 +293,7 @@ TEST(
cache->insertInterestHandle(object4->getHash(), object4);
cache->insertInterestHandle(object5->getHash(), object5);
EXPECT_EQ(12, cache->getStats().totalSizeInBytes);
EXPECT_EQ(12, cache->getTotalSizeBytes());
EXPECT_TRUE(cache->getInterestHandle(hash3).object);
EXPECT_TRUE(cache->getInterestHandle(hash4).object);
EXPECT_TRUE(cache->getInterestHandle(hash5).object);
@ -308,7 +308,7 @@ TEST(ObjectCache, interest_handle_preserves_minimum_number_of_entries) {
cache->insertInterestHandle(object5->getHash(), object5);
cache->insertInterestHandle(object6->getHash(), object6);
EXPECT_EQ(15, cache->getStats().totalSizeInBytes);
EXPECT_EQ(15, cache->getTotalSizeBytes());
EXPECT_FALSE(cache->getInterestHandle(hash3).object);
EXPECT_TRUE(cache->getInterestHandle(hash4).object);
EXPECT_TRUE(cache->getInterestHandle(hash5).object);
@ -457,11 +457,11 @@ TEST(ObjectCache, interest_handle_redundant_inserts_are_ignored) {
create(10, 0, makeRefPtr<EdenStats>());
auto object = std::make_shared<CacheObject>(ObjectId{}, 9);
cache->insertInterestHandle(object->getHash(), object);
EXPECT_EQ(9, cache->getStats().totalSizeInBytes);
EXPECT_EQ(9, cache->getTotalSizeBytes());
cache->insertInterestHandle(object->getHash(), object);
EXPECT_EQ(9, cache->getStats().totalSizeInBytes);
EXPECT_EQ(9, cache->getTotalSizeBytes());
cache->insertInterestHandle(object->getHash(), object);
EXPECT_EQ(9, cache->getStats().totalSizeInBytes);
EXPECT_EQ(9, cache->getTotalSizeBytes());
}
TEST(