Skip to content

Commit

Permalink
Make MemoryAllocator into a Customizable class (facebook#8980)
Browse files Browse the repository at this point in the history
Summary:
- Make MemoryAllocator and its implementations into a Customizable class.
- Added a "DefaultMemoryAllocator" which uses new and delete
- Added a "CountedMemoryAllocator" that counts the number of allocs and free
- Updated the existing tests to use these new allocators
- Changed the memkind allocator test into a generic test that can test the various allocators.
- Added tests for creating all of the allocators
- Added tests to verify/create the JemallocNodumpAllocator using its options.

Pull Request resolved: facebook#8980

Reviewed By: zhichao-cao

Differential Revision: D32990403

Pulled By: mrambacher

fbshipit-source-id: 6fdfe8218c10dd8dfef34344a08201be1fa95c76
  • Loading branch information
mrambacher authored and facebook-github-bot committed Dec 17, 2021
1 parent 9828b6d commit 423538a
Show file tree
Hide file tree
Showing 21 changed files with 739 additions and 305 deletions.
3 changes: 2 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ set(SOURCES
memory/concurrent_arena.cc
memory/jemalloc_nodump_allocator.cc
memory/memkind_kmem_allocator.cc
memory/memory_allocator.cc
memtable/alloc_tracker.cc
memtable/hash_linklist_rep.cc
memtable/hash_skiplist_rep.cc
Expand Down Expand Up @@ -1270,7 +1271,7 @@ if(WITH_TESTS)
logging/env_logger_test.cc
logging/event_logger_test.cc
memory/arena_test.cc
memory/memkind_kmem_allocator_test.cc
memory/memory_allocator_test.cc
memtable/inlineskiplist_test.cc
memtable/skiplist_test.cc
memtable/write_buffer_manager_test.cc
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ db_repl_stress: $(OBJ_DIR)/tools/db_repl_stress.o $(LIBRARY)
arena_test: $(OBJ_DIR)/memory/arena_test.o $(TEST_LIBRARY) $(LIBRARY)
$(AM_LINK)

memkind_kmem_allocator_test: memory/memkind_kmem_allocator_test.o $(TEST_LIBRARY) $(LIBRARY)
memory_allocator_test: memory/memory_allocator_test.o $(TEST_LIBRARY) $(LIBRARY)
$(AM_LINK)

autovector_test: $(OBJ_DIR)/util/autovector_test.o $(TEST_LIBRARY) $(LIBRARY)
Expand Down
6 changes: 4 additions & 2 deletions TARGETS
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ cpp_library(
"memory/concurrent_arena.cc",
"memory/jemalloc_nodump_allocator.cc",
"memory/memkind_kmem_allocator.cc",
"memory/memory_allocator.cc",
"memtable/alloc_tracker.cc",
"memtable/hash_linklist_rep.cc",
"memtable/hash_skiplist_rep.cc",
Expand Down Expand Up @@ -586,6 +587,7 @@ cpp_library(
"memory/concurrent_arena.cc",
"memory/jemalloc_nodump_allocator.cc",
"memory/memkind_kmem_allocator.cc",
"memory/memory_allocator.cc",
"memtable/alloc_tracker.cc",
"memtable/hash_linklist_rep.cc",
"memtable/hash_skiplist_rep.cc",
Expand Down Expand Up @@ -1781,8 +1783,8 @@ ROCKS_TESTS = [
[],
],
[
"memkind_kmem_allocator_test",
"memory/memkind_kmem_allocator_test.cc",
"memory_allocator_test",
"memory/memory_allocator_test.cc",
"parallel",
[],
[],
Expand Down
4 changes: 2 additions & 2 deletions db/db_impl/db_impl_write.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1660,8 +1660,8 @@ Status DBImpl::TrimMemtableHistory(WriteContext* context) {
}
for (auto& cfd : cfds) {
autovector<MemTable*> to_delete;
bool trimmed = cfd->imm()->TrimHistory(
&context->memtables_to_free_, cfd->mem()->MemoryAllocatedBytes());
bool trimmed = cfd->imm()->TrimHistory(&context->memtables_to_free_,
cfd->mem()->MemoryAllocatedBytes());
if (trimmed) {
context->superversion_context.NewSuperVersion();
assert(context->superversion_context.new_superversion.get() != nullptr);
Expand Down
4 changes: 2 additions & 2 deletions db/memtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ class MemTable {

// used by MemTableListVersion::MemoryAllocatedBytesExcludingLast
size_t MemoryAllocatedBytes() const {
return table_->ApproximateMemoryUsage() +
range_del_table_->ApproximateMemoryUsage() +
return table_->ApproximateMemoryUsage() +
range_del_table_->ApproximateMemoryUsage() +
arena_.MemoryAllocatedBytes();
}

Expand Down
8 changes: 4 additions & 4 deletions db/memtable_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,8 @@ size_t MemTableList::ApproximateUnflushedMemTablesMemoryUsage() {
size_t MemTableList::ApproximateMemoryUsage() { return current_memory_usage_; }

size_t MemTableList::MemoryAllocatedBytesExcludingLast() const {
const size_t usage =
current_memory_allocted_bytes_excluding_last_.load(std::memory_order_relaxed);
const size_t usage = current_memory_allocted_bytes_excluding_last_.load(
std::memory_order_relaxed);
return usage;
}

Expand All @@ -610,8 +610,8 @@ bool MemTableList::HasHistory() const {
void MemTableList::UpdateCachedValuesFromMemTableListVersion() {
const size_t total_memtable_size =
current_->MemoryAllocatedBytesExcludingLast();
current_memory_allocted_bytes_excluding_last_.store(total_memtable_size,
std::memory_order_relaxed);
current_memory_allocted_bytes_excluding_last_.store(
total_memtable_size, std::memory_order_relaxed);

const bool has_history = current_->HasHistory();
current_has_history_.store(has_history, std::memory_order_relaxed);
Expand Down
6 changes: 3 additions & 3 deletions db/memtable_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,9 @@ class MemTableList {
// Returns the cached current_has_history_ value.
bool HasHistory() const;

// Updates current_memory_allocted_bytes_excluding_last_ and current_has_history_
// from MemTableListVersion. Must be called whenever InstallNewVersion is
// called.
// Updates current_memory_allocted_bytes_excluding_last_ and
// current_has_history_ from MemTableListVersion. Must be called whenever
// InstallNewVersion is called.
void UpdateCachedValuesFromMemTableListVersion();

// `usage` is the current size of the mutable Memtable. When
Expand Down
4 changes: 2 additions & 2 deletions db_stress_tool/db_stress_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ UniqueIdVerifier::UniqueIdVerifier(const std::string& db_name)

Status st = fs.CreateDirIfMissing(db_name, opts, nullptr);
if (!st.ok()) {
fprintf(stderr, "Failed to create directory %s: %s\n",
db_name.c_str(), st.ToString().c_str());
fprintf(stderr, "Failed to create directory %s: %s\n", db_name.c_str(),
st.ToString().c_str());
exit(1);
}

Expand Down
18 changes: 11 additions & 7 deletions include/rocksdb/memory_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,23 @@

#pragma once

#include "rocksdb/status.h"

#include <memory>

#include "rocksdb/customizable.h"
#include "rocksdb/status.h"

namespace ROCKSDB_NAMESPACE {

// MemoryAllocator is an interface that a client can implement to supply custom
// memory allocation and deallocation methods. See rocksdb/cache.h for more
// information.
// All methods should be thread-safe.
class MemoryAllocator {
class MemoryAllocator : public Customizable {
public:
virtual ~MemoryAllocator() = default;

// Name of the cache allocator, printed in the log
virtual const char* Name() const = 0;
static const char* Type() { return "MemoryAllocator"; }
static Status CreateFromString(const ConfigOptions& options,
const std::string& value,
std::shared_ptr<MemoryAllocator>* result);

// Allocate a block of at least size. Has to be thread-safe.
virtual void* Allocate(size_t size) = 0;
Expand All @@ -34,9 +35,12 @@ class MemoryAllocator {
// default implementation just returns the allocation size
return allocation_size;
}

std::string GetId() const override { return GenerateIndividualId(); }
};

struct JemallocAllocatorOptions {
static const char* kName() { return "JemallocAllocatorOptions"; }
// Jemalloc tcache cache allocations by size class. For each size class,
// it caches between 20 (for large size classes) to 200 (for small size
// classes). To reduce tcache memory usage in case the allocator is access
Expand Down
Loading

0 comments on commit 423538a

Please sign in to comment.