From 3a0e86395e60a08921ae3a687f933f0fe61c2f83 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Tue, 9 May 2017 21:26:22 -0700 Subject: [PATCH] Convert eviction code to STL (#534) * temp commit * convert eviction policy to C++ * temp commit * fix plasma tests * fix * linting * fixes * fix linting --- src/plasma/eviction_policy.cc | 199 ++++++++++++---------------------- src/plasma/test/run_tests.sh | 2 +- 2 files changed, 72 insertions(+), 129 deletions(-) diff --git a/src/plasma/eviction_policy.cc b/src/plasma/eviction_policy.cc index d25cc39cf..f1c1acb0b 100644 --- a/src/plasma/eviction_policy.cc +++ b/src/plasma/eviction_policy.cc @@ -1,109 +1,64 @@ #include "eviction_policy.h" -#include "utlist.h" +#include +#include -/** An element representing a released object in a doubly-linked list. This is - * used to implement an LRU cache. */ -typedef struct ReleasedObject { - /** The object_id of the released object. */ - ObjectID object_id; - /** Needed for the doubly-linked list macros. */ - struct ReleasedObject *prev; - /** Needed for the doubly-linked list macros. */ - struct ReleasedObject *next; -} ReleasedObject; +class LRUCache { + private: + /** A doubly-linked list containing the items in the cache and + * their sizes in LRU order. */ + typedef std::list> ItemList; + ItemList item_list_; + /** A hash table mapping the object ID of an object in the cache to its + * location in the doubly linked list item_list_. */ + std::unordered_map item_map_; -/** This type is used to define a hash table mapping the object ID of a released - * object to its location in the doubly-linked list of released objects. */ -typedef struct { - /** Object ID of this object. */ - ObjectID object_id; - /** A pointer to the corresponding entry for this object in the doubly-linked - * list of released objects. */ - ReleasedObject *released_object; - /** Handle for the uthash table. */ - UT_hash_handle handle; -} released_object_entry; + public: + LRUCache(){}; + + void add(const ObjectID &key, int64_t size) { + auto it = item_map_.find(key); + CHECK(it == item_map_.end()); + item_list_.push_front(std::make_pair(key, size)); + item_map_.insert(std::make_pair(key, item_list_.begin())); + } + + void remove(const ObjectID &key) { + auto it = item_map_.find(key); + CHECK(it != item_map_.end()); + item_list_.erase(it->second); + item_map_.erase(it); + } + + int64_t choose_objects_to_evict(int64_t num_bytes_required, + std::vector &objects_to_evict) { + int64_t bytes_evicted = 0; + auto it = item_list_.end(); + while (bytes_evicted < num_bytes_required && it != item_list_.begin()) { + it--; + objects_to_evict.push_back(it->first); + bytes_evicted += it->second; + } + return bytes_evicted; + } +}; /** The part of the Plasma state that is maintained by the eviction policy. */ struct EvictionState { /** The amount of memory (in bytes) currently being used. */ int64_t memory_used; - /** A doubly-linked list of the released objects in order from least recently - * released to most recently released. */ - ReleasedObject *released_objects; - /** A hash table mapping the object ID of a released object to its location in - * the doubly linked list of released objects. */ - released_object_entry *released_object_table; + /** Datastructure for the LRU cache. */ + LRUCache cache; }; -/* This is used to define the array of object IDs used to define the - * released_objects type. */ -UT_icd released_objects_entry_icd = {sizeof(ObjectID), NULL, NULL, NULL}; - -EvictionState *EvictionState_init(void) { - EvictionState *state = (EvictionState *) malloc(sizeof(EvictionState)); - state->memory_used = 0; - state->released_objects = NULL; - state->released_object_table = NULL; - return state; +EvictionState *EvictionState_init() { + EvictionState *s = new EvictionState(); + s->memory_used = 0; + return s; } void EvictionState_free(EvictionState *s) { - /* Delete each element in the doubly-linked list. */ - ReleasedObject *element, *temp; - DL_FOREACH_SAFE(s->released_objects, element, temp) { - DL_DELETE(s->released_objects, element); - free(element); - } - /* Delete each element in the hash table. */ - released_object_entry *current_entry, *temp_entry; - HASH_ITER(handle, s->released_object_table, current_entry, temp_entry) { - HASH_DELETE(handle, s->released_object_table, current_entry); - free(current_entry); - } - /* Free the eviction state. */ - free(s); -} - -void add_object_to_lru_cache(EvictionState *eviction_state, - ObjectID object_id) { - /* Add the object ID to the doubly-linked list. */ - ReleasedObject *linked_list_entry = - (ReleasedObject *) malloc(sizeof(ReleasedObject)); - linked_list_entry->object_id = object_id; - DL_APPEND(eviction_state->released_objects, linked_list_entry); - /* Check that the object ID is not already in the hash table. */ - released_object_entry *hash_table_entry; - HASH_FIND(handle, eviction_state->released_object_table, &object_id, - sizeof(object_id), hash_table_entry); - CHECK(hash_table_entry == NULL); - /* Add the object ID to the hash table. */ - hash_table_entry = - (released_object_entry *) malloc(sizeof(released_object_entry)); - hash_table_entry->object_id = object_id; - hash_table_entry->released_object = linked_list_entry; - HASH_ADD(handle, eviction_state->released_object_table, object_id, - sizeof(object_id), hash_table_entry); -} - -void remove_object_from_lru_cache(EvictionState *eviction_state, - ObjectID object_id) { - /* Check that the object ID is in the hash table. */ - released_object_entry *hash_table_entry; - HASH_FIND(handle, eviction_state->released_object_table, &object_id, - sizeof(object_id), hash_table_entry); - /* Only remove the object ID if it is in the LRU cache. */ - CHECK(hash_table_entry != NULL); - /* Remove the object ID from the doubly-linked list. */ - DL_DELETE(eviction_state->released_objects, - hash_table_entry->released_object); - /* Free the entry from the doubly-linked list. */ - free(hash_table_entry->released_object); - /* Remove the object ID from the hash table. */ - HASH_DELETE(handle, eviction_state->released_object_table, hash_table_entry); - /* Free the entry from the hash table. */ - free(hash_table_entry); + delete s; } int64_t EvictionState_choose_objects_to_evict( @@ -112,47 +67,33 @@ int64_t EvictionState_choose_objects_to_evict( int64_t num_bytes_required, int64_t *num_objects_to_evict, ObjectID **objects_to_evict) { - int64_t num_objects = 0; - int64_t num_bytes = 0; - /* Figure out how many objects need to be evicted in order to recover a - * sufficient number of bytes. */ - ReleasedObject *element, *temp; - DL_FOREACH_SAFE(eviction_state->released_objects, element, temp) { - if (num_bytes >= num_bytes_required) { - break; - } - /* Find the object table entry for this object. */ - ObjectTableEntry *entry = plasma_store_info->objects[element->object_id]; - /* Update the cumulative bytes and the number of objects so far. */ - num_bytes += (entry->info.data_size + entry->info.metadata_size); - num_objects += 1; + std::vector objs_to_evict; + int64_t bytes_evicted = eviction_state->cache.choose_objects_to_evict( + num_bytes_required, objs_to_evict); + /* Update the LRU cache. */ + for (auto &object_id : objs_to_evict) { + eviction_state->cache.remove(object_id); } /* Construct the return values. */ - *num_objects_to_evict = num_objects; - if (num_objects == 0) { + *num_objects_to_evict = objs_to_evict.size(); + if (objs_to_evict.size() == 0) { *objects_to_evict = NULL; } else { - *objects_to_evict = (ObjectID *) malloc(num_objects * sizeof(ObjectID)); - int counter = 0; - DL_FOREACH_SAFE(eviction_state->released_objects, element, temp) { - if (counter == num_objects) { - break; - } - (*objects_to_evict)[counter] = element->object_id; - /* Update the LRU cache. */ - remove_object_from_lru_cache(eviction_state, element->object_id); - counter += 1; - } + int64_t result_size = objs_to_evict.size() * sizeof(ObjectID); + *objects_to_evict = (ObjectID *) malloc(result_size); + memcpy(*objects_to_evict, objs_to_evict.data(), result_size); } - /* Update the number used. */ - eviction_state->memory_used -= num_bytes; - return num_bytes; + /* Update the number of bytes used. */ + eviction_state->memory_used -= bytes_evicted; + return bytes_evicted; } void EvictionState_object_created(EvictionState *eviction_state, PlasmaStoreInfo *plasma_store_info, - ObjectID obj_id) { - add_object_to_lru_cache(eviction_state, obj_id); + ObjectID object_id) { + ObjectTableEntry *entry = plasma_store_info->objects[object_id]; + eviction_state->cache.add(object_id, + entry->info.data_size + entry->info.metadata_size); } bool EvictionState_require_space(EvictionState *eviction_state, @@ -192,22 +133,24 @@ bool EvictionState_require_space(EvictionState *eviction_state, void EvictionState_begin_object_access(EvictionState *eviction_state, PlasmaStoreInfo *plasma_store_info, - ObjectID obj_id, + ObjectID object_id, int64_t *num_objects_to_evict, ObjectID **objects_to_evict) { /* If the object is in the LRU cache, remove it. */ - remove_object_from_lru_cache(eviction_state, obj_id); + eviction_state->cache.remove(object_id); *num_objects_to_evict = 0; *objects_to_evict = NULL; } void EvictionState_end_object_access(EvictionState *eviction_state, PlasmaStoreInfo *plasma_store_info, - ObjectID obj_id, + ObjectID object_id, int64_t *num_objects_to_evict, ObjectID **objects_to_evict) { + ObjectTableEntry *entry = plasma_store_info->objects[object_id]; /* Add the object to the LRU cache.*/ - add_object_to_lru_cache(eviction_state, obj_id); + eviction_state->cache.add(object_id, + entry->info.data_size + entry->info.metadata_size); *num_objects_to_evict = 0; *objects_to_evict = NULL; } diff --git a/src/plasma/test/run_tests.sh b/src/plasma/test/run_tests.sh index eb66bec78..ccb1da562 100644 --- a/src/plasma/test/run_tests.sh +++ b/src/plasma/test/run_tests.sh @@ -13,7 +13,7 @@ killall plasma_store redis_pid=$! sleep 1 # flush the redis server -../common/thirdparty/redis/src/redis-cli flushall & +./src/common/thirdparty/redis/src/redis-cli flushall & sleep 1 ./src/plasma/plasma_store -s /tmp/store1 -m 1000000000 & plasma1_pid=$!