search

intel / CacheLib

Pluggable in-process caching engine to build and scale high performance services
https://www.cachelib.org
Apache License 2.0
17 stars 4 forks source link

Per tier pool stats #70

Closed byrnedj closed 1 year ago

byrnedj commented 1 year ago

This PR adds per tier statistics to the pool stats. We also introduce a new statistic called writebacks which counts the number of successfully moved items to the next tier.

Cachebench now outputs per tier hit ratios and per tier item counts so we can get a better idea of how much data is being served from each tier.

I have verified that there is no impact to performance with this PR.

This change is Reviewable

vinser52 commented 1 year ago

Can we add some tests for per-tier stats?

igchor commented 1 year ago

In general looks good. Can you paste here some example output with those stats?

guptask commented 1 year ago

cachelib/allocator/Cache.cpp line 239 at r1 (raw file):


  //TODO: per-tier
  const auto stats = getGlobalCacheStats();

Does this change mean currently these starts are only aggregated for tier 0 ?

guptask commented 1 year ago

cachelib/allocator/CacheAllocator-inl.h line 2799 at r1 (raw file):

      MMContainerStat mmContainerStats;
      for (TierId tid = 0; tid < getNumTiers(); tid++) {
        allocAttempts += (*stats_.allocAttempts)[tid][poolId][cid].get();

Just wondering is it possible to implement a + operator override for these stats ? I'm thinking about the scenario where we add more stats - it would make the code cleaner since that operator would be within the class.

guptask commented 1 year ago

cachelib/allocator/CacheAllocator-inl.h line 2799 at r1 (raw file):

Previously, guptask (Sounak Gupta) wrote…
Just wondering is it possible to implement a + operator override for these stats ? I'm thinking about the scenario where we add more stats - it would make the code cleaner since that operator would be within the class.

Or maybe std::accumulate() ?

guptask commented 1 year ago

cachelib/allocator/CacheStats.h line 454 at r1 (raw file):


  // size of itemRemoved_ hash set in nvm
  uint64_t numNvmItemRemovedSetSize{0};

Minor: Instead of having vectors for all per-tier stats, I think it would be cleaner to encapsulate those as one class and a vector object for that class inside global cache stats.

byrnedj commented 1 year ago

Can we add some tests for per-tier stats?

I have added in my latest commit.

byrnedj commented 1 year ago

In general looks good. Can you paste here some example output with those stats?

image