From 3a6922276a140a90468d2a8489f1722242de2007 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Thu, 11 May 2017 11:18:23 -0700 Subject: [PATCH] convert malloc.c to STL (#537) * convert malloc.c to STL * linting * cleanup and comments * address Richard's comments --- src/plasma/CMakeLists.txt | 2 +- src/plasma/{malloc.c => malloc.cc} | 65 +++++++++++++++--------------- src/plasma/plasma_store.cc | 4 +- 3 files changed, 35 insertions(+), 36 deletions(-) rename src/plasma/{malloc.c => malloc.cc} (70%) diff --git a/src/plasma/CMakeLists.txt b/src/plasma/CMakeLists.txt index 879464807..a35af93f6 100644 --- a/src/plasma/CMakeLists.txt +++ b/src/plasma/CMakeLists.txt @@ -73,7 +73,7 @@ add_executable(plasma_store plasma_protocol.cc eviction_policy.cc fling.c - malloc.c) + malloc.cc) add_dependencies(plasma_store hiredis gen_plasma_fbs) diff --git a/src/plasma/malloc.c b/src/plasma/malloc.cc similarity index 70% rename from src/plasma/malloc.c rename to src/plasma/malloc.cc index a88d4d969..e8399649a 100644 --- a/src/plasma/malloc.c +++ b/src/plasma/malloc.cc @@ -6,9 +6,11 @@ #include #include -#include "common.h" -#include "uthash.h" +#include +#include "common.h" + +extern "C" { void *fake_mmap(size_t); int fake_munmap(void *, size_t); @@ -30,20 +32,23 @@ int fake_munmap(void *, size_t); #undef USE_DL_PREFIX #undef HAVE_MORECORE #undef DEFAULT_GRANULARITY +} struct mmap_record { int fd; - void *pointer; int64_t size; - UT_hash_handle hh_fd; - UT_hash_handle hh_pointer; }; -/* TODO(rshin): Don't have two hash tables. */ -struct mmap_record *records_by_fd = NULL; -struct mmap_record *records_by_pointer = NULL; +namespace { -const int GRANULARITY_MULTIPLIER = 2; +/** Hashtable that contains one entry per segment that we got from the OS + * via mmap. Associates the address of that segment with its file descriptor + * and size. */ +std::unordered_map mmap_records; + +} /* namespace */ + +constexpr int GRANULARITY_MULTIPLIER = 2; static void *pointer_advance(void *p, ptrdiff_t n) { return (unsigned char *) p + n; @@ -69,12 +74,12 @@ int create_buffer(int64_t size) { } #else #ifdef __linux__ - static char template[] = "/dev/shm/plasmaXXXXXX"; + constexpr char file_template[] = "/dev/shm/plasmaXXXXXX"; #else - static char template[] = "/tmp/plasmaXXXXXX"; + constexpr char file_template[] = "/tmp/plasmaXXXXXX"; #endif char file_name[32]; - strncpy(file_name, template, 32); + strncpy(file_name, file_template, 32); fd = mkstemp(file_name); if (fd < 0) return -1; @@ -111,12 +116,9 @@ void *fake_mmap(size_t size) { /* Increase dlmalloc's allocation granularity directly. */ mparams.granularity *= GRANULARITY_MULTIPLIER; - struct mmap_record *record = malloc(sizeof(struct mmap_record)); - record->fd = fd; - record->pointer = pointer; - record->size = size; - HASH_ADD(hh_fd, records_by_fd, fd, sizeof(fd), record); - HASH_ADD(hh_pointer, records_by_pointer, pointer, sizeof(pointer), record); + mmap_record &record = mmap_records[pointer]; + record.fd = fd; + record.size = size; /* We lie to dlmalloc about where mapped memory actually lives. */ pointer = pointer_advance(pointer, sizeof(size_t)); @@ -129,22 +131,20 @@ int fake_munmap(void *addr, size_t size) { addr = pointer_retreat(addr, sizeof(size_t)); size += sizeof(size_t); - struct mmap_record *record; + auto entry = mmap_records.find(addr); - HASH_FIND(hh_pointer, records_by_pointer, &addr, sizeof(addr), record); - if (record == NULL || record->size != size) { + if (entry == mmap_records.end() || entry->second.size != size) { /* Reject requests to munmap that don't directly match previous * calls to mmap, to prevent dlmalloc from trimming. */ return -1; } - HASH_DELETE(hh_fd, records_by_fd, record); - HASH_DELETE(hh_pointer, records_by_pointer, record); - int r = munmap(addr, size); if (r == 0) { - close(record->fd); + close(entry->second.fd); } + + mmap_records.erase(entry); return r; } @@ -152,14 +152,13 @@ void get_malloc_mapinfo(void *addr, int *fd, int64_t *map_size, ptrdiff_t *offset) { - struct mmap_record *record; - /* TODO(rshin): Implement a more efficient search through records_by_fd. */ - for (record = records_by_fd; record != NULL; record = record->hh_fd.next) { - if (addr >= record->pointer && - addr < pointer_advance(record->pointer, record->size)) { - *fd = record->fd; - *map_size = record->size; - *offset = pointer_distance(record->pointer, addr); + /* TODO(rshin): Implement a more efficient search through mmap_records. */ + for (const auto &entry : mmap_records) { + if (addr >= entry.first && + addr < pointer_advance(entry.first, entry.second.size)) { + *fd = entry.second.fd; + *map_size = entry.second.size; + *offset = pointer_distance(entry.first, addr); return; } } diff --git a/src/plasma/plasma_store.cc b/src/plasma/plasma_store.cc index 2512dcad7..431205d0e 100644 --- a/src/plasma/plasma_store.cc +++ b/src/plasma/plasma_store.cc @@ -31,17 +31,17 @@ #include #include "common.h" -#include "format/common_generated.h" #include "event_loop.h" #include "eviction_policy.h" +#include "format/common_generated.h" #include "io.h" +#include "malloc.h" #include "plasma_protocol.h" #include "plasma_store.h" #include "plasma.h" extern "C" { #include "fling.h" -#include "malloc.h" void *dlmalloc(size_t); void *dlmemalign(size_t alignment, size_t bytes); void dlfree(void *);