Closed igchor closed 2 years ago
Hi @igchor, most of the metrics have the name and value separated by a single colon like: 'Hit Ratio : 92.07%' That metric appears in prometheus named 'hit_ratio'
For the line below, I'm thinking how we would parse this output into 'name, value' for prometheus tid : 0, pid : 0, cid : 0, alloc size : 64.00B, memory size : 0.00B, free : 100.00%
If we want to follow the same format as other metrics maybe we could use this format: tid0 pid0 cid0 64.00KB size : 0.00B tid0 pid0 cid0 64.00KB free : 100.00%
Does it make sense to change the stats output to use this format or something like this? Our existing scripts will automatically collect new metrics output in this format.
cachelib/allocator/CacheAllocator-inl.h
line 2510 at r1 (raw file):
template <typename CacheTrait> double CacheAllocator<CacheTrait>::slabsFreePercentage(TierId tid) const
should this name have approx term in it as well ?
cachelib/allocator/memory/AllocationClass.cpp
line 54 at r1 (raw file):
slabAlloc_(s), freedAllocations_{slabAlloc_.createSingleTierPtrCompressor<FreeAlloc>()} { curAllocatedSlabs_ = allocatedSlabs_.size();
Do you want to use memory_order_seq_cst order here for atomic store ? Otherwise store() is needed if you want to use something like memory_order_release order.
cachelib/allocator/memory/AllocationClass.cpp
line 126 at r1 (raw file):
} curAllocatedSlabs_ = allocatedSlabs_.size();
same as above comment about memory order if using assignment operator instead of atomic store().
cachelib/allocator/memory/AllocationClass.cpp
line 741 at r1 (raw file):
double AllocationClass::approxFreePercentage() const { if (getNumSlabs() == 0)
Cosmetic review : I suggest using curly braces even if there is one line after if statement.
cachelib/allocator/tests/CacheBaseTest.cpp
line 36 at r1 (raw file):
const MemoryPool& getPool(PoolId) const override { return memoryPool_; } PoolStats getPoolStats(PoolId) const override { return PoolStats(); } AllocationClassBaseStat getAllocationClassStats(TierId tid, PoolId, ClassId) const {return AllocationClassBaseStat();};
am not sure this line will pass the clang-format check. the character count is high.
@raema Ok, I changed it to how you suggested. Please let me now if this is OK.
This output looks good. Thanks!
Printing those stats can be enabled by passing
-report_memory_usage_stats
to cachebenchSample output:
This change is