You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2018/03/23 05:11:51 UTC
[5/6] kudu git commit: KUDU-972. Cache should track the memory
overhead
KUDU-972. Cache should track the memory overhead
This changes the cache 'charge' mechanism to use malloc_usable_size on
its actual allocations rather than tracking only the cached _values_.
This addresses two sources of inaccuracy:
1) we weren't counting the cache key and LRUHandle overhead, which is
apparently 104 bytes per object.
2) we weren't accounting for tcmalloc internal fragmentation, which can
be significant for allocations in our typical block size range. For
example, a 4097-byte allocation will use 4608 bytes (12% overhead). In
an insert-heavy workload most of hte cache is used for bloom blocks
which are ~4kb, so the above is a very common case.
Change-Id: I6cc69004dccb1cc372fdedbac321154e46a92263
Reviewed-on: http://gerrit.cloudera.org:8080/9752
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/9df14d36
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/9df14d36
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/9df14d36
Branch: refs/heads/master
Commit: 9df14d362487c97de2d5e46cb190296c188f66e9
Parents: 600723e
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Mar 21 19:24:58 2018 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Mar 23 00:51:49 2018 +0000
----------------------------------------------------------------------
src/kudu/cfile/block_cache-test.cc | 15 ++++++++++++++
src/kudu/cfile/block_cache.cc | 35 +++++++++++++++++++++------------
src/kudu/cfile/block_cache.h | 4 ++++
src/kudu/util/cache.cc | 3 ++-
src/kudu/util/cache.h | 15 ++++++++++++++
src/kudu/util/nvm_cache.cc | 3 ++-
6 files changed, 60 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/9df14d36/src/kudu/cfile/block_cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/block_cache-test.cc b/src/kudu/cfile/block_cache-test.cc
index 86acbef..b34f0b1 100644
--- a/src/kudu/cfile/block_cache-test.cc
+++ b/src/kudu/cfile/block_cache-test.cc
@@ -16,11 +16,15 @@
// under the License.
#include <cstring>
+#include <memory>
+#include <ostream>
+#include <glog/logging.h>
#include <gtest/gtest.h>
#include "kudu/cfile/block_cache.h"
#include "kudu/util/cache.h"
+#include "kudu/util/mem_tracker.h"
#include "kudu/util/slice.h"
namespace kudu {
@@ -34,6 +38,11 @@ TEST(TestBlockCache, TestBasics) {
BlockCache::FileId id(1234);
BlockCache::CacheKey key(id, 1);
+ std::shared_ptr<MemTracker> mem_tracker;
+ if (BlockCache::GetConfiguredCacheTypeOrDie() == DRAM_CACHE) {
+ ASSERT_TRUE(MemTracker::FindTracker("block_cache-sharded_lru_cache", &mem_tracker));
+ }
+
// Lookup something missing from cache
{
BlockCacheHandle handle;
@@ -50,6 +59,12 @@ TEST(TestBlockCache, TestBasics) {
ASSERT_FALSE(data.valid());
ASSERT_TRUE(inserted_handle.valid());
+ if (mem_tracker) {
+ int overhead = mem_tracker->consumption() - data_size;
+ EXPECT_GT(overhead, 0);
+ LOG(INFO) << "Cache overhead for one entry: " << overhead;
+ }
+
BlockCacheHandle retrieved_handle;
ASSERT_TRUE(cache.Lookup(key, Cache::EXPECT_IN_CACHE, &retrieved_handle));
ASSERT_TRUE(retrieved_handle.valid());
http://git-wip-us.apache.org/repos/asf/kudu/blob/9df14d36/src/kudu/cfile/block_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/block_cache.cc b/src/kudu/cfile/block_cache.cc
index 9a4f034..12ab3bb 100644
--- a/src/kudu/cfile/block_cache.cc
+++ b/src/kudu/cfile/block_cache.cc
@@ -15,12 +15,17 @@
// specific language governing permissions and limitations
// under the License.
+#include <cstddef>
+#include <cstdint>
#include <ostream>
#include <string>
#include <gflags/gflags.h>
+#include <glog/logging.h>
#include "kudu/cfile/block_cache.h"
+#include "kudu/gutil/gscoped_ptr.h"
+#include "kudu/gutil/macros.h"
#include "kudu/util/cache.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/slice.h"
@@ -47,21 +52,26 @@ namespace cfile {
namespace {
Cache* CreateCache(int64_t capacity) {
- CacheType t;
- ToUpperCase(FLAGS_block_cache_type, &FLAGS_block_cache_type);
- if (FLAGS_block_cache_type == "NVM") {
- t = NVM_CACHE;
- } else if (FLAGS_block_cache_type == "DRAM") {
- t = DRAM_CACHE;
- } else {
- LOG(FATAL) << "Unknown block cache type: '" << FLAGS_block_cache_type
- << "' (expected 'DRAM' or 'NVM')";
- }
+ CacheType t = BlockCache::GetConfiguredCacheTypeOrDie();
return NewLRUCache(t, capacity, "block_cache");
}
} // anonymous namespace
+CacheType BlockCache::GetConfiguredCacheTypeOrDie() {
+ ToUpperCase(FLAGS_block_cache_type, &FLAGS_block_cache_type);
+ if (FLAGS_block_cache_type == "NVM") {
+ return NVM_CACHE;
+ }
+ if (FLAGS_block_cache_type == "DRAM") {
+ return DRAM_CACHE;
+ }
+
+ LOG(FATAL) << "Unknown block cache type: '" << FLAGS_block_cache_type
+ << "' (expected 'DRAM' or 'NVM')";
+ __builtin_unreachable();
+}
+
BlockCache::BlockCache()
: BlockCache(FLAGS_block_cache_capacity_mb * 1024 * 1024) {
}
@@ -70,10 +80,9 @@ BlockCache::BlockCache(size_t capacity)
: cache_(CreateCache(capacity)) {
}
-BlockCache::PendingEntry BlockCache::Allocate(const CacheKey& key, size_t val_size) {
+BlockCache::PendingEntry BlockCache::Allocate(const CacheKey& key, size_t block_size) {
Slice key_slice(reinterpret_cast<const uint8_t*>(&key), sizeof(key));
- int charge = val_size;
- return PendingEntry(cache_.get(), cache_->Allocate(key_slice, val_size, charge));
+ return PendingEntry(cache_.get(), cache_->Allocate(key_slice, block_size));
}
bool BlockCache::Lookup(const CacheKey& key, Cache::CacheBehavior behavior,
http://git-wip-us.apache.org/repos/asf/kudu/blob/9df14d36/src/kudu/cfile/block_cache.h
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/block_cache.h b/src/kudu/cfile/block_cache.h
index 83f5aff..2122522 100644
--- a/src/kudu/cfile/block_cache.h
+++ b/src/kudu/cfile/block_cache.h
@@ -48,6 +48,10 @@ class BlockCacheHandle;
// Provides a singleton and LRU cache for CFile blocks.
class BlockCache {
public:
+ // Parse the gflag which configures the block cache. FATALs if the flag is
+ // invalid.
+ static CacheType GetConfiguredCacheTypeOrDie();
+
// BlockId refers to the unique identifier for a Kudu block, that is, for an
// entire CFile. This is different than the block cache's notion of a block,
// which is just a portion of a CFile.
http://git-wip-us.apache.org/repos/asf/kudu/blob/9df14d36/src/kudu/util/cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/cache.cc b/src/kudu/util/cache.cc
index a2fc80b..1fe11b2 100644
--- a/src/kudu/util/cache.cc
+++ b/src/kudu/util/cache.cc
@@ -30,6 +30,7 @@
#include "kudu/util/cache_metrics.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/locks.h"
+#include "kudu/util/malloc.h"
#include "kudu/util/mem_tracker.h"
#include "kudu/util/metrics.h"
#include "kudu/util/slice.h"
@@ -489,7 +490,7 @@ class ShardedLRUCache : public Cache {
LRUHandle* handle = reinterpret_cast<LRUHandle*>(buf);
handle->key_length = key_len;
handle->val_length = val_len;
- handle->charge = charge;
+ handle->charge = (charge == kAutomaticCharge) ? kudu_malloc_usable_size(buf) : charge;
handle->hash = HashSlice(key);
memcpy(handle->kv_data, key.data(), key_len);
http://git-wip-us.apache.org/repos/asf/kudu/blob/9df14d36/src/kudu/util/cache.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/cache.h b/src/kudu/util/cache.h
index 462307c..82ef8c9 100644
--- a/src/kudu/util/cache.h
+++ b/src/kudu/util/cache.h
@@ -157,12 +157,21 @@ class Cache {
// the cache.
struct PendingHandle { };
+ // Indicates that the charge of an item in the cache should be calculated
+ // based on its memory consumption.
+ static constexpr int kAutomaticCharge = -1;
+
// Allocate space for a new entry to be inserted into the cache.
//
// The provided 'key' is copied into the resulting handle object.
// The allocated handle has enough space such that the value can
// be written into cache_->MutableValue(handle).
//
+ // If 'charge' is not 'kAutomaticCharge', then the cache capacity will be charged
+ // the explicit amount. This is useful when caching items that are small but need to
+ // maintain a bounded count (eg file descriptors) rather than caring about their actual
+ // memory usage.
+ //
// Note that this does not mutate the cache itself: lookups will
// not be able to find the provided key until it is inserted.
//
@@ -173,6 +182,12 @@ class Cache {
// caller must either free it using Free(), or insert it using Insert().
virtual PendingHandle* Allocate(Slice key, int val_len, int charge) = 0;
+ // Default 'charge' should be kAutomaticCharge.
+ // (default arguments on virtual functions are prohibited)
+ PendingHandle* Allocate(Slice key, int val_len) {
+ return Allocate(key, val_len, kAutomaticCharge);
+ }
+
virtual uint8_t* MutableValue(PendingHandle* handle) = 0;
// Commit a prepared entry into the cache.
http://git-wip-us.apache.org/repos/asf/kudu/blob/9df14d36/src/kudu/util/nvm_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/nvm_cache.cc b/src/kudu/util/nvm_cache.cc
index ae8a871..ef52ec0 100644
--- a/src/kudu/util/nvm_cache.cc
+++ b/src/kudu/util/nvm_cache.cc
@@ -541,7 +541,8 @@ class ShardedLRUCache : public Cache {
handle->kv_data = &buf[sizeof(LRUHandle)];
handle->val_length = val_len;
handle->key_length = key_len;
- handle->charge = charge + key.size();
+ handle->charge = (charge == kAutomaticCharge) ?
+ vmem_malloc_usable_size(vmp_, buf) : charge;
handle->hash = HashSlice(key);
memcpy(handle->kv_data, key.data(), key.size());
return reinterpret_cast<PendingHandle*>(handle);