From ad4cbd40164d5b79bd8ddae36bd6d46f701352fc Mon Sep 17 00:00:00 2001 From: Peter Schafhalter Date: Fri, 20 Oct 2017 10:06:22 -0700 Subject: [PATCH] Updated outstanding_callbacks to unordered_map (#1108) * Updated outstanding_callbacks to unordered_map * Fix bug in destroy_outstanding_callbacks and comments --- src/common/state/table.cc | 41 +++++++++++++++++++++++---------------- src/common/state/table.h | 9 +++------ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/common/state/table.cc b/src/common/state/table.cc index 3db3d2e45..6c678f051 100644 --- a/src/common/state/table.cc +++ b/src/common/state/table.cc @@ -1,5 +1,6 @@ #include "table.h" +#include #include #include "redis.h" @@ -117,9 +118,9 @@ int64_t table_timeout_handler(event_loop *loop, } /** - * Hash table maintaining the outstanding callbacks. + * Unordered map maintaining the outstanding callbacks. * - * This hash table is used to handle the following case: + * This unordered map is used to handle the following case: * - a table command is issued with an associated callback and a callback data * structure; * - the last timeout associated to this command expires, as a result the @@ -127,16 +128,16 @@ int64_t table_timeout_handler(event_loop *loop, * - a reply arrives, but now the callback data structure is gone, so we have * to ignore this reply; * - * This hash table enables us to ignore such replies. The operations on the - * hash table are as follows. + * This unordered map enables us to ignore such replies. The operations on the + * unordered map are as follows. * * When we issue a table command and a timeout event to wait for the reply, we - * add a new entry to the hash table that is keyed by the ID of the timer. Note - * that table commands must have unique timer IDs, which are assigned by the - * Redis ae event loop. + * add a new entry to the unordered map that is keyed by the ID of the timer. + * Note that table commands must have unique timer IDs, which are assigned by + * the Redis ae event loop. * * When we receive the reply, we check whether the callback still exists in - * this hash table, and if not we just ignore the reply. If the callback does + * this unordered map, and if not we just ignore the reply. If the callback does * exist, the reply receiver is responsible for removing the timer and the * entry associated to the callback, or else the timeout handler will continue * firing. @@ -144,25 +145,31 @@ int64_t table_timeout_handler(event_loop *loop, * When the last timeout associated to the command expires we remove the entry * associated to the callback. */ -static TableCallbackData *outstanding_callbacks = NULL; +static std::unordered_map outstanding_callbacks; void outstanding_callbacks_add(TableCallbackData *callback_data) { - HASH_ADD_INT(outstanding_callbacks, timer_id, callback_data); + outstanding_callbacks[callback_data->timer_id] = callback_data; } TableCallbackData *outstanding_callbacks_find(int64_t key) { - TableCallbackData *callback_data = NULL; - HASH_FIND_INT(outstanding_callbacks, &key, callback_data); - return callback_data; + auto it = outstanding_callbacks.find(key); + if (it != outstanding_callbacks.end()) { + return it->second; + } + return NULL; } void outstanding_callbacks_remove(TableCallbackData *callback_data) { - HASH_DEL(outstanding_callbacks, callback_data); + outstanding_callbacks.erase(callback_data->timer_id); } void destroy_outstanding_callbacks(event_loop *loop) { - TableCallbackData *callback_data, *tmp; - HASH_ITER(hh, outstanding_callbacks, callback_data, tmp) { - destroy_timer_callback(loop, callback_data); + /* We have to be careful because destroy_timer_callback modifies + * outstanding_callbacks in place */ + auto it = outstanding_callbacks.begin(); + while (it != outstanding_callbacks.end()) { + auto next_it = std::next(it, 1); + destroy_timer_callback(loop, it->second); + it = next_it; } } diff --git a/src/common/state/table.h b/src/common/state/table.h index 5c247dd3e..9674eb782 100644 --- a/src/common/state/table.h +++ b/src/common/state/table.h @@ -1,8 +1,6 @@ #ifndef TABLE_H #define TABLE_H -#include "uthash.h" - #include "common.h" #include "db.h" @@ -70,7 +68,6 @@ struct TableCallbackData { DBHandle *db_handle; /** Handle to timer. */ int64_t timer_id; - UT_hash_handle hh; /* makes this structure hashable */ }; /** @@ -114,7 +111,7 @@ TableCallbackData *init_table_callback(DBHandle *db_handle, /** * Destroy any state associated with the callback data. This removes all - * associated state from the outstanding callbacks hash table and frees any + * associated state from the outstanding callbacks unordered map and frees any * associated memory. This does not remove any associated timer events. * * @param callback_data The pointer to the data structure of the callback we @@ -155,7 +152,7 @@ void outstanding_callbacks_add(TableCallbackData *callback_data); /** * Find an outstanding callback entry. * - * @param key The key for the outstanding callbacks hash table. We use the + * @param key The key for the outstanding callbacks unordered map. We use the * timer ID assigned by the Redis ae event loop. * @return Returns the callback data if found, NULL otherwise. */ @@ -163,7 +160,7 @@ TableCallbackData *outstanding_callbacks_find(int64_t key); /** * Remove an outstanding callback entry. This only removes the callback entry - * from the hash table. It does not free the entry or remove any associated + * from the unordered map. It does not free the entry or remove any associated * timer events. * * @param callback_data The pointer to the data structure of the callback we