From e3d97fc6750612de32defd46946ebdb415fba896 Mon Sep 17 00:00:00 2001 From: Kevin Yang Date: Fri, 20 Jan 2017 17:03:07 -0800 Subject: [PATCH 1/3] Add data structures, interface, and basic unit tests for hotkey sampling/detection --- src/CMakeLists.txt | 1 + src/hotkey/CMakeLists.txt | 6 + src/hotkey/constant.h | 3 + src/hotkey/counter_table.c | 242 ++++++++++++++++++ src/hotkey/counter_table.h | 20 ++ src/hotkey/hotkey.c | 79 ++++++ src/hotkey/hotkey.h | 36 +++ src/hotkey/queue.c | 190 ++++++++++++++ src/hotkey/queue.h | 16 ++ test/CMakeLists.txt | 1 + test/hotkey/#check_queue.c# | 0 test/hotkey/.#check_queue.c | 1 + test/hotkey/CMakeLists.txt | 2 + test/hotkey/counter_table/CMakeLists.txt | 11 + .../counter_table/check_counter_table.c | 99 +++++++ test/hotkey/queue/CMakeLists.txt | 11 + test/hotkey/queue/check_queue.c | 133 ++++++++++ test/protocol/admin/check_admin.c | 2 +- 18 files changed, 852 insertions(+), 1 deletion(-) create mode 100644 src/hotkey/CMakeLists.txt create mode 100644 src/hotkey/constant.h create mode 100644 src/hotkey/counter_table.c create mode 100644 src/hotkey/counter_table.h create mode 100644 src/hotkey/hotkey.c create mode 100644 src/hotkey/hotkey.h create mode 100644 src/hotkey/queue.c create mode 100644 src/hotkey/queue.h create mode 100644 test/hotkey/#check_queue.c# create mode 120000 test/hotkey/.#check_queue.c create mode 100644 test/hotkey/CMakeLists.txt create mode 100644 test/hotkey/counter_table/CMakeLists.txt create mode 100644 test/hotkey/counter_table/check_counter_table.c create mode 100644 test/hotkey/queue/CMakeLists.txt create mode 100644 test/hotkey/queue/check_queue.c diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 667df663f..47a420826 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,4 +1,5 @@ add_subdirectory(core ${PROJECT_BINARY_DIR}/core) +add_subdirectory(hotkey ${PROJECT_BINARY_DIR}/hotkey) add_subdirectory(protocol ${PROJECT_BINARY_DIR}/protocol) add_subdirectory(storage ${PROJECT_BINARY_DIR}/storage) add_subdirectory(time ${PROJECT_BINARY_DIR}/time) diff --git a/src/hotkey/CMakeLists.txt b/src/hotkey/CMakeLists.txt new file mode 100644 index 000000000..3f10e251b --- /dev/null +++ b/src/hotkey/CMakeLists.txt @@ -0,0 +1,6 @@ +set(SOURCE + counter_table.c + hotkey.c + queue.c) + +add_library(hotkey ${SOURCE}) diff --git a/src/hotkey/constant.h b/src/hotkey/constant.h new file mode 100644 index 000000000..1cde643f0 --- /dev/null +++ b/src/hotkey/constant.h @@ -0,0 +1,3 @@ +#pragma once + +#define MAX_KEY_LEN 250 diff --git a/src/hotkey/counter_table.c b/src/hotkey/counter_table.c new file mode 100644 index 000000000..3d9bb9af4 --- /dev/null +++ b/src/hotkey/counter_table.c @@ -0,0 +1,242 @@ +#include "counter_table.h" + +#include "constant.h" + +#include +#include +#include +#include + +#define COUNTER_TABLE_MODULE_NAME "hotkey::counter_table" + +struct counter_table_entry { + STAILQ_ENTRY(counter_table_entry) next; /* entry in hash table or pool */ + + char key[MAX_KEY_LEN]; + uint32_t nkey; + uint32_t count; +}; + +STAILQ_HEAD(cte_slh, counter_table_entry); + +static struct cte_slh *table = NULL; +static uint32_t table_size = 0; /* number of buckets in table */ +static bool counter_table_init = false; + +FREEPOOL(cte_pool, cteq, counter_table_entry); +static struct cte_pool ctep; +static bool ctep_init = false; + +static void +counter_table_entry_reset(struct counter_table_entry *cte) +{ + cte->nkey = 0; + cte->count = 0; +} + +static struct counter_table_entry * +counter_table_entry_create(void) +{ + struct counter_table_entry *cte = cc_alloc(sizeof(*cte)); + + if (cte == NULL) { + return NULL; + } + + counter_table_entry_reset(cte); + + return cte; +} + +static void +counter_table_entry_destroy(struct counter_table_entry **counter_table_entry) +{ + struct counter_table_entry *cte = *counter_table_entry; + ASSERT(cte != NULL); + + cc_free(cte); + *counter_table_entry = NULL; +} + +static void +counter_table_entry_pool_destroy(void) +{ + struct counter_table_entry *cte, *tcte; + + if (!ctep_init) { + log_warn("counter_table_entry pool was not created, ignore"); + } + + log_info("destroying counter_table_entry pool: free %"PRIu32, ctep.nfree); + + FREEPOOL_DESTROY(cte, tcte, &ctep, next, counter_table_entry_destroy); + ctep_init = false; +} + +static struct counter_table_entry * +counter_table_entry_borrow(void) +{ + struct counter_table_entry *cte; + + FREEPOOL_BORROW(cte, &ctep, next, counter_table_entry_create); + if (cte == NULL) { + log_debug("borrow counter_table_entry failed: OOM"); + return NULL; + } + counter_table_entry_reset(cte); + + return cte; +} + +static void +counter_table_entry_return(struct counter_table_entry **counter_table_entry) +{ + struct counter_table_entry *cte = *counter_table_entry; + + if (cte == NULL) { + return; + } + + FREEPOOL_RETURN(cte, &ctep, next); + + *counter_table_entry = NULL; +} + +static void +counter_table_entry_pool_create(uint32_t max) +{ + struct counter_table_entry *cte; + + if (ctep_init) { + log_warn("counter_table_entry pool has already been created, re-creating"); + counter_table_entry_pool_destroy(); + } + + log_info("creating counter_table_entry pool: max %"PRIu32, max); + + FREEPOOL_CREATE(&ctep, max); + ctep_init = true; + + FREEPOOL_PREALLOC(cte, &ctep, max, next, counter_table_entry_create); + if (ctep.nfree < max) { + log_crit("cannot preallocate counter_table_entry pool, OOM. abort"); + exit(EXIT_FAILURE); + } +} + +void +counter_table_setup(uint32_t size, uint32_t poolsize) +{ + uint32_t i; + + log_info("Set up the %s module", COUNTER_TABLE_MODULE_NAME); + + if (counter_table_init) { + log_warn("%s has already been setup, ignore", COUNTER_TABLE_MODULE_NAME); + return; + } + + table = cc_alloc(sizeof(*table) * size); + table_size = size; + + if (table == NULL) { + log_crit("Could not allocate counter table for hotkey - OOM"); + exit(EXIT_FAILURE); + } + + for (i = 0; i < size; ++i) { + STAILQ_INIT(&table[i]); + } + + counter_table_entry_pool_create(poolsize); +} + +void +counter_table_teardown(void) +{ + log_info("Tear down the %s module", COUNTER_TABLE_MODULE_NAME); + + if (!counter_table_init) { + log_warn("%s was not setup", COUNTER_TABLE_MODULE_NAME); + } + + if (table != NULL) { + uint32_t i; + /* free all entries in table */ + for (i = 0; i < table_size; ++i) { + struct counter_table_entry *cte, *tcte; + STAILQ_FOREACH_SAFE(cte, &(table[i]), next, tcte) { + counter_table_entry_return(&cte); + } + } + } + + counter_table_entry_pool_destroy(); + counter_table_init = false; +} + +static inline struct cte_slh * +_get_bucket(const char *key, uint32_t nkey) +{ + return &(table[hash(key, nkey, 0) % table_size]); +} + +uint32_t +counter_table_incr(char *key, uint32_t nkey) +{ + struct counter_table_entry *cte; + struct cte_slh *bucket; + + ASSERT(nkey <= MAX_KEY_LEN); + + bucket = _get_bucket(key, nkey); + + /* iterate through bucket looking for item */ + for (cte = STAILQ_FIRST(bucket); cte != NULL; cte = STAILQ_NEXT(cte, next)) { + if ((nkey == cte->nkey) && cc_memcmp(key, cte->key, nkey) == 0) { + /* found item */ + return ++cte->count; + } + } + + /* not found, insert entry */ + cte = counter_table_entry_borrow(); + cc_memcpy(cte->key, key, nkey); + cte->nkey = nkey; + cte->count = 1; + STAILQ_INSERT_HEAD(bucket, cte, next); + + return 1; +} + +void +counter_table_decr(char *key, uint32_t nkey) +{ + struct counter_table_entry *cte, *prev; + struct cte_slh *bucket; + + ASSERT(nkey <= MAX_KEY_LEN); + + bucket = _get_bucket(key, nkey); + + /* iterate through bucket looking for item */ + for (prev = NULL, cte = STAILQ_FIRST(bucket); cte != NULL; + prev = cte, cte = STAILQ_NEXT(cte, next)) { + if ((nkey == cte->nkey) && cc_memcmp(key, cte->key, nkey) == 0) { + /* found item */ + if (--(cte->count) == 0) { + /* remove entry */ + if (prev == NULL) { + STAILQ_REMOVE_HEAD(bucket, next); + } else { + STAILQ_REMOVE_AFTER(bucket, prev, next); + } + } + + return; + } + } + + /* item not found, should never happen for hotkey detection scheme */ + ASSERT(false); +} diff --git a/src/hotkey/counter_table.h b/src/hotkey/counter_table.h new file mode 100644 index 000000000..d084400ba --- /dev/null +++ b/src/hotkey/counter_table.h @@ -0,0 +1,20 @@ +#pragma once + +#include + +/* + * The counter_table module provides a utility for counting the frequency at + * which keys appear. When a key is sampled, it should be incremented in the + * table after enqueueing it. When a key is dequeued, it should be decremented + * from the table. + */ + +/* setup/teardown module */ +void counter_table_setup(uint32_t size, uint32_t poolsize); +void counter_table_teardown(void); + +/* increment count and return count, insert into table if count == 0 */ +uint32_t counter_table_incr(char *key, uint32_t nkey); + +/* decrement count, remove from table if count == 0 after decr */ +void counter_table_decr(char *key, uint32_t nkey); diff --git a/src/hotkey/hotkey.c b/src/hotkey/hotkey.c new file mode 100644 index 000000000..a925c0abb --- /dev/null +++ b/src/hotkey/hotkey.c @@ -0,0 +1,79 @@ +#include "hotkey.h" + +#include "constant.h" +#include "counter_table.h" +#include "queue.h" + +#include + +#define HOTKEY_MODULE_NAME "hotkey::hotkey" + +bool hotkey_enabled = false; + +static uint64_t hotkey_counter; + +static bool hotkey_init = false; +static uint32_t hotkey_nsample = HOTKEY_NSAMPLE; +static uint32_t hotkey_rate = HOTKEY_RATE; +static uint32_t hotkey_threshold = HOTKEY_THRESHOLD; +static uint32_t hotkey_nsample_cur = 0; + +void +hotkey_setup(hotkey_options_st *options) +{ + log_info("Set up the %s module", HOTKEY_MODULE_NAME); + + if (options != NULL) { + hotkey_enabled = option_bool(&options->hotkey_enable); + hotkey_nsample = option_uint(&options->hotkey_sample_size); + hotkey_rate = option_uint(&options->hotkey_sample_rate); + hotkey_threshold = option_uint(&options->hotkey_threshold); + } + + hotkey_nsample_cur = 0; + hotkey_counter = 0; + queue_setup(hotkey_nsample); + /* TODO: determine whether table size should be a tuneable parameter */ + counter_table_setup(hotkey_nsample, hotkey_nsample); + hotkey_init = true; +} + +void +hotkey_teardown(void) +{ + log_info("Tear down the %s module", HOTKEY_MODULE_NAME); + + if (!hotkey_init) { + log_warn("%s was not setup", HOTKEY_MODULE_NAME); + } + + hotkey_enabled = false; + queue_teardown(); + counter_table_teardown(); + hotkey_init = false; +} + +bool +_hotkey_sample(char *key, uint32_t nkey) +{ + if (++hotkey_counter % hotkey_rate == 0) { + /* sample this key */ + uint32_t freq; + + if (queue_len() == hotkey_nsample) { + char buf[MAX_KEY_LEN]; + uint32_t len; + + /* pop from queue, decrement in counter table */ + len = queue_pop(buf); + counter_table_decr(buf, len); + } + + queue_push(key, nkey); + freq = counter_table_incr(key, nkey); + + return freq >= hotkey_threshold; + } + + return false; +} diff --git a/src/hotkey/hotkey.h b/src/hotkey/hotkey.h new file mode 100644 index 000000000..be3e39ed5 --- /dev/null +++ b/src/hotkey/hotkey.h @@ -0,0 +1,36 @@ +#pragma once + +#include + +#include +#include + +/* TODO(kevyang): add stats for hotkey module */ + +#define HOTKEY_NSAMPLE 10000 /* keep last 10000 keys sampled by default */ +#define HOTKEY_RATE 100 /* sample one in every 100 keys by default */ +#define HOTKEY_THRESHOLD 10 /* signal for hotkey if 10 or more keys in sample by default */ + +/* name type default description */ +#define HOTKEY_OPTION(ACTION) \ + ACTION( hotkey_enable, OPTION_TYPE_BOOL, false, "use hotkey detection?" )\ + ACTION( hotkey_sample_size, OPTION_TYPE_UINT, HOTKEY_NSAMPLE, "number of keys to maintain" )\ + ACTION( hotkey_sample_rate, OPTION_TYPE_UINT, HOTKEY_RATE, "hotkey sample ratio" )\ + ACTION( hotkey_threshold, OPTION_TYPE_UINT, HOTKEY_THRESHOLD, "threshold for hotkey signal") + +typedef struct { + HOTKEY_OPTION(OPTION_DECLARE) +} hotkey_options_st; + +extern bool hotkey_enabled; + +void hotkey_setup(hotkey_options_st *options); +void hotkey_teardown(void); + +#define hotkey_sample(key, nkey) do { \ + if (hotkey_enabled) { \ + _hotkey_sample(key, nkey); \ + } \ +} while (0) + +bool _hotkey_sample(char *key, uint32_t nkey); diff --git a/src/hotkey/queue.c b/src/hotkey/queue.c new file mode 100644 index 000000000..f40d0845d --- /dev/null +++ b/src/hotkey/queue.c @@ -0,0 +1,190 @@ +#include "queue.h" +#include "constant.h" + +#include +#include +#include + +#define QUEUE_MODULE_NAME "hotkey::queue" + +static bool queue_init = false; +static int queue_size = 0; + +FREEPOOL(qn_pool, qnq, queue_node); +static struct qn_pool qnp; +static bool qnp_init = false; + +struct queue_node { + char key[MAX_KEY_LEN]; + uint32_t nkey; + STAILQ_ENTRY(queue_node) next; +}; + +STAILQ_HEAD(queue, queue_node); +struct queue q = STAILQ_HEAD_INITIALIZER(q); + +static void +queue_node_reset(struct queue_node *qn) +{ + qn->nkey = 0; +} + +static struct queue_node * +queue_node_create(void) +{ + struct queue_node *qn = cc_alloc(sizeof(*qn)); + + if (qn == NULL) { + return NULL; + } + + queue_node_reset(qn); + + return qn; +} + +static void +queue_node_destroy(struct queue_node **queue_node) +{ + struct queue_node *qn = *queue_node; + ASSERT(qn != NULL); + + cc_free(qn); + *queue_node = NULL; +} + +static void +queue_node_pool_destroy(void) +{ + struct queue_node *qn, *tqn; + + if (!qnp_init) { + log_warn("queue_node pool was not created, ignore"); + } + + log_info("destroying queue_node pool: free %"PRIu32, qnp.nfree); + + FREEPOOL_DESTROY(qn, tqn, &qnp, next, queue_node_destroy); + qnp_init = false; +} + +static void +queue_node_pool_create(uint32_t max) +{ + struct queue_node *qn; + + if (qnp_init) { + log_warn("queue_node pool has already been created, re-creating"); + queue_node_pool_destroy(); + } + + log_info("creating queue_node pool: max %"PRIu32, max); + + FREEPOOL_CREATE(&qnp, max); + qnp_init = true; + + FREEPOOL_PREALLOC(qn, &qnp, max, next, queue_node_create); + if (qnp.nfree < max) { + log_crit("cannot preallocate queue_node pool, OOM. abort"); + exit(EXIT_FAILURE); + } +} + +static struct queue_node * +queue_node_borrow(void) +{ + struct queue_node *qn; + + FREEPOOL_BORROW(qn, &qnp, next, queue_node_create); + if (qn == NULL) { + log_debug("borrow queue_node failed: OOM"); + return NULL; + } + queue_node_reset(qn); + + return qn; +} + +static void +queue_node_return(struct queue_node **queue_node) +{ + struct queue_node *qn = *queue_node; + + if (qn == NULL) { + return; + } + + FREEPOOL_RETURN(qn, &qnp, next); + + *queue_node = NULL; +} + +void +queue_setup(uint32_t poolsize) +{ + log_info("set up the %s module", QUEUE_MODULE_NAME); + + if (queue_init) { + log_warn("%s has already been setup, overwrite", QUEUE_MODULE_NAME); + } + + queue_node_pool_create(poolsize); + queue_size = 0; + STAILQ_INIT(&q); + queue_init = true; +} + +void +queue_teardown(void) +{ + struct queue_node *qn, *tqn; + + log_info("tear down the %s module", QUEUE_MODULE_NAME); + + if (!queue_init) { + log_warn("%s was not setup", QUEUE_MODULE_NAME); + } + + /* free all entries in queue */ + STAILQ_FOREACH_SAFE(qn, &q, next, tqn) { + queue_node_return(&qn); + } + + queue_node_pool_destroy(); + queue_init = false; +} + +void +queue_push(char *key, uint32_t nkey) +{ + struct queue_node *qn = queue_node_borrow(); + + ASSERT(nkey <= MAX_KEY_LEN); + + cc_memcpy(qn->key, key, nkey); + qn->nkey = nkey; + STAILQ_INSERT_TAIL(&q, qn, next); + ++queue_size; +} + +uint32_t +queue_pop(char *buf) +{ + struct queue_node *qn = STAILQ_FIRST(&q); + uint32_t nkey; + + cc_memcpy(buf, qn->key, qn->nkey); + nkey = qn->nkey; + + STAILQ_REMOVE_HEAD(&q, next); + queue_node_return(&qn); + --queue_size; + + return nkey; +} + +uint32_t +queue_len(void) +{ + return queue_size; +} diff --git a/src/hotkey/queue.h b/src/hotkey/queue.h new file mode 100644 index 000000000..d165f6796 --- /dev/null +++ b/src/hotkey/queue.h @@ -0,0 +1,16 @@ +#pragma once + +#include + +/* + * The queue module provides a FIFO queue interface to help with bookkeeping the + * last N keys provided for hotkey detection. + */ + +void queue_push(char *key, uint32_t nkey); +uint32_t queue_pop(char *buf); /* returns length of key */ +uint32_t queue_len(void); + +/* Setup/teardown functions, must be called before module is used */ +void queue_setup(uint32_t poolsize); +void queue_teardown(void); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 420dde343..42b6bead5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -3,4 +3,5 @@ include_directories(${include_directories} CHECK_INCLUDES) add_custom_target(check COMMAND ${CMAKE_CTEST_COMMAND}) add_subdirectory(protocol) +add_subdirectory(hotkey) add_subdirectory(storage) diff --git a/test/hotkey/#check_queue.c# b/test/hotkey/#check_queue.c# new file mode 100644 index 000000000..e69de29bb diff --git a/test/hotkey/.#check_queue.c b/test/hotkey/.#check_queue.c new file mode 120000 index 000000000..628b1e49b --- /dev/null +++ b/test/hotkey/.#check_queue.c @@ -0,0 +1 @@ +kyang@tw-mbp-kyang.67519 \ No newline at end of file diff --git a/test/hotkey/CMakeLists.txt b/test/hotkey/CMakeLists.txt new file mode 100644 index 000000000..9cc2a02ca --- /dev/null +++ b/test/hotkey/CMakeLists.txt @@ -0,0 +1,2 @@ +add_subdirectory(counter_table) +add_subdirectory(queue) diff --git a/test/hotkey/counter_table/CMakeLists.txt b/test/hotkey/counter_table/CMakeLists.txt new file mode 100644 index 000000000..d4c0802d1 --- /dev/null +++ b/test/hotkey/counter_table/CMakeLists.txt @@ -0,0 +1,11 @@ +set(suite counter_table) +set(test_name check_${suite}) + +set(source check_${suite}.c) + +add_executable(${test_name} ${source}) +target_link_libraries(${test_name} hotkey) +target_link_libraries(${test_name} ccommon-static ${CHECK_LIBRARIES}) + +add_dependencies(check ${test_name}) +add_test(${test_name} ${test_name}) diff --git a/test/hotkey/counter_table/check_counter_table.c b/test/hotkey/counter_table/check_counter_table.c new file mode 100644 index 000000000..d27b9f5a5 --- /dev/null +++ b/test/hotkey/counter_table/check_counter_table.c @@ -0,0 +1,99 @@ +#include + +#include + +#include + +#include + +#define SUITE_NAME "counter_table" +#define DEBUG_LOG SUITE_NAME ".log" + +#define TEST_TABLE_SIZE 10 + +/* + * utilities + */ +static void +test_setup(void) +{ + counter_table_setup(TEST_TABLE_SIZE, TEST_TABLE_SIZE); +} + +static void +test_teardown(void) +{ + counter_table_teardown(); +} + +static void +test_reset(void) +{ + test_teardown(); + test_setup(); +} + +/************** + * test cases * + **************/ + +START_TEST(test_basic) +{ + char *key1 = "key1", *key2 = "key2"; + uint32_t klen = 4; + uint32_t count; + + test_reset(); + + count = counter_table_incr(key1, klen); + ck_assert_int_eq(count, 1); + + count = counter_table_incr(key2, klen); + ck_assert_int_eq(count, 1); + + count = counter_table_incr(key1, klen); + ck_assert_int_eq(count, 2); + + counter_table_decr(key1, klen); + count = counter_table_incr(key1, klen); + ck_assert_int_eq(count, 2); +} +END_TEST + +/* + * test suite + */ +static Suite * +counter_table_suite(void) +{ + Suite *s = suite_create(SUITE_NAME); + + /* basic functionality */ + TCase *tc_basic_counter_table = tcase_create("basic counter_table"); + suite_add_tcase(s, tc_basic_counter_table); + + tcase_add_test(tc_basic_counter_table, test_basic); + + return s; +} + +int +main(void) +{ + int nfail; + + /* setup */ + test_setup(); + + Suite *suite = counter_table_suite(); + SRunner *srunner = srunner_create(suite); + srunner_set_log(srunner, DEBUG_LOG); + srunner_run_all(srunner, CK_ENV); /* set CK_VERBOSITY in ENV to customize */ + nfail = srunner_ntests_failed(srunner); + srunner_free(srunner); + + /* teardown */ + test_teardown(); + + return (nfail == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} diff --git a/test/hotkey/queue/CMakeLists.txt b/test/hotkey/queue/CMakeLists.txt new file mode 100644 index 000000000..b1402543e --- /dev/null +++ b/test/hotkey/queue/CMakeLists.txt @@ -0,0 +1,11 @@ +set(suite queue) +set(test_name check_${suite}) + +set(source check_${suite}.c) + +add_executable(${test_name} ${source}) +target_link_libraries(${test_name} hotkey) +target_link_libraries(${test_name} ccommon-static ${CHECK_LIBRARIES}) + +add_dependencies(check ${test_name}) +add_test(${test_name} ${test_name}) diff --git a/test/hotkey/queue/check_queue.c b/test/hotkey/queue/check_queue.c new file mode 100644 index 000000000..c2e95d80e --- /dev/null +++ b/test/hotkey/queue/check_queue.c @@ -0,0 +1,133 @@ +#include + +#include + +#include + +#include +#include +#include + +#define SUITE_NAME "queue" +#define DEBUG_LOG SUITE_NAME ".log" + +#define TEST_QUEUE_SIZE 10 + +/* + * utilities + */ +static void +test_setup(void) +{ + queue_setup(TEST_QUEUE_SIZE); +} + +static void +test_teardown(void) +{ + queue_teardown(); +} + +static void +test_reset(void) +{ + test_teardown(); + test_setup(); +} + +/************** + * test cases * + **************/ + +START_TEST(test_basic) +{ + char *key = "key1"; + uint32_t klen = 4; + char buf[MAX_KEY_LEN]; + uint32_t queue_pop_len; + + test_reset(); + + ck_assert_int_eq(queue_len(), 0); + + queue_push(key, klen); + ck_assert_int_eq(queue_len(), 1); + queue_pop_len = queue_pop(buf); + + ck_assert_int_eq(queue_len(), 0); + ck_assert_int_eq(queue_pop_len, klen); + ck_assert(strncmp(key, buf, klen) == 0); +} +END_TEST + +START_TEST(test_multiple) +{ + char *key1 = "key1", *key2 = "key22", *key3 = "key333"; + uint32_t klen1 = 4, klen2 = 5, klen3 = 6; + char buf[MAX_KEY_LEN]; + uint32_t queue_pop_len; + + test_reset(); + + ck_assert_int_eq(queue_len(), 0); + + queue_push(key1, klen1); + queue_push(key2, klen2); + queue_push(key3, klen3); + ck_assert_int_eq(queue_len(), 3); + + queue_pop_len = queue_pop(buf); + ck_assert_int_eq(queue_pop_len, klen1); + ck_assert(strncmp(key1, buf, klen1) == 0); + ck_assert_int_eq(queue_len(), 2); + + queue_pop_len = queue_pop(buf); + ck_assert_int_eq(queue_pop_len, klen2); + ck_assert(strncmp(key2, buf, klen2) == 0); + ck_assert_int_eq(queue_len(), 1); + + queue_pop_len = queue_pop(buf); + ck_assert_int_eq(queue_pop_len, klen3); + ck_assert(strncmp(key3, buf, klen3) == 0); + ck_assert_int_eq(queue_len(), 0); +} +END_TEST + +/* + * test suite + */ +static Suite * +queue_suite(void) +{ + Suite *s = suite_create(SUITE_NAME); + + /* basic queue functionality */ + TCase *tc_basic_queue = tcase_create("basic queue"); + suite_add_tcase(s, tc_basic_queue); + + tcase_add_test(tc_basic_queue, test_basic); + tcase_add_test(tc_basic_queue, test_multiple); + + return s; +} + +int +main(void) +{ + int nfail; + + /* setup */ + test_setup(); + + Suite *suite = queue_suite(); + SRunner *srunner = srunner_create(suite); + srunner_set_log(srunner, DEBUG_LOG); + srunner_run_all(srunner, CK_ENV); /* set CK_VERBOSITY in ENV to customize */ + nfail = srunner_ntests_failed(srunner); + srunner_free(srunner); + + /* teardown */ + test_teardown(); + + return (nfail == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} diff --git a/test/protocol/admin/check_admin.c b/test/protocol/admin/check_admin.c index c72a1cb69..4c0edc90e 100644 --- a/test/protocol/admin/check_admin.c +++ b/test/protocol/admin/check_admin.c @@ -147,7 +147,7 @@ main(void) Suite *suite = admin_suite(); SRunner *srunner = srunner_create(suite); srunner_set_log(srunner, DEBUG_LOG); - srunner_run_all(srunner, CK_ENV); /* set CK_VEBOSITY in ENV to customize */ + srunner_run_all(srunner, CK_ENV); /* set CK_VERBOSITY in ENV to customize */ nfail = srunner_ntests_failed(srunner); srunner_free(srunner); From a8ecabbb05a723111e0f99721a51f558f7ded554 Mon Sep 17 00:00:00 2001 From: Kevin Yang Date: Fri, 17 Feb 2017 12:43:12 -0800 Subject: [PATCH 2/3] address comments --- src/hotkey/CMakeLists.txt | 4 +- src/hotkey/counter_table.c | 242 ----------------- src/hotkey/counter_table.h | 20 -- src/hotkey/hotkey.c | 42 +-- src/hotkey/hotkey.h | 30 +-- src/hotkey/kc_map.c | 244 ++++++++++++++++++ src/hotkey/kc_map.h | 22 ++ src/hotkey/key_window.c | 195 ++++++++++++++ src/hotkey/key_window.h | 18 ++ src/hotkey/queue.c | 190 -------------- src/hotkey/queue.h | 16 -- test/hotkey/#check_queue.c# | 0 test/hotkey/.#check_queue.c | 1 - test/hotkey/CMakeLists.txt | 4 +- test/hotkey/{queue => kc_map}/CMakeLists.txt | 2 +- .../check_kc_map.c} | 40 +-- .../CMakeLists.txt | 2 +- test/hotkey/key_window/check_key_window.c | 145 +++++++++++ test/hotkey/queue/check_queue.c | 133 ---------- 19 files changed, 689 insertions(+), 661 deletions(-) delete mode 100644 src/hotkey/counter_table.c delete mode 100644 src/hotkey/counter_table.h create mode 100644 src/hotkey/kc_map.c create mode 100644 src/hotkey/kc_map.h create mode 100644 src/hotkey/key_window.c create mode 100644 src/hotkey/key_window.h delete mode 100644 src/hotkey/queue.c delete mode 100644 src/hotkey/queue.h delete mode 100644 test/hotkey/#check_queue.c# delete mode 120000 test/hotkey/.#check_queue.c rename test/hotkey/{queue => kc_map}/CMakeLists.txt (94%) rename test/hotkey/{counter_table/check_counter_table.c => kc_map/check_kc_map.c} (60%) rename test/hotkey/{counter_table => key_window}/CMakeLists.txt (91%) create mode 100644 test/hotkey/key_window/check_key_window.c delete mode 100644 test/hotkey/queue/check_queue.c diff --git a/src/hotkey/CMakeLists.txt b/src/hotkey/CMakeLists.txt index 3f10e251b..cd137ef01 100644 --- a/src/hotkey/CMakeLists.txt +++ b/src/hotkey/CMakeLists.txt @@ -1,6 +1,6 @@ set(SOURCE - counter_table.c hotkey.c - queue.c) + kc_map.c + key_window.c) add_library(hotkey ${SOURCE}) diff --git a/src/hotkey/counter_table.c b/src/hotkey/counter_table.c deleted file mode 100644 index 3d9bb9af4..000000000 --- a/src/hotkey/counter_table.c +++ /dev/null @@ -1,242 +0,0 @@ -#include "counter_table.h" - -#include "constant.h" - -#include -#include -#include -#include - -#define COUNTER_TABLE_MODULE_NAME "hotkey::counter_table" - -struct counter_table_entry { - STAILQ_ENTRY(counter_table_entry) next; /* entry in hash table or pool */ - - char key[MAX_KEY_LEN]; - uint32_t nkey; - uint32_t count; -}; - -STAILQ_HEAD(cte_slh, counter_table_entry); - -static struct cte_slh *table = NULL; -static uint32_t table_size = 0; /* number of buckets in table */ -static bool counter_table_init = false; - -FREEPOOL(cte_pool, cteq, counter_table_entry); -static struct cte_pool ctep; -static bool ctep_init = false; - -static void -counter_table_entry_reset(struct counter_table_entry *cte) -{ - cte->nkey = 0; - cte->count = 0; -} - -static struct counter_table_entry * -counter_table_entry_create(void) -{ - struct counter_table_entry *cte = cc_alloc(sizeof(*cte)); - - if (cte == NULL) { - return NULL; - } - - counter_table_entry_reset(cte); - - return cte; -} - -static void -counter_table_entry_destroy(struct counter_table_entry **counter_table_entry) -{ - struct counter_table_entry *cte = *counter_table_entry; - ASSERT(cte != NULL); - - cc_free(cte); - *counter_table_entry = NULL; -} - -static void -counter_table_entry_pool_destroy(void) -{ - struct counter_table_entry *cte, *tcte; - - if (!ctep_init) { - log_warn("counter_table_entry pool was not created, ignore"); - } - - log_info("destroying counter_table_entry pool: free %"PRIu32, ctep.nfree); - - FREEPOOL_DESTROY(cte, tcte, &ctep, next, counter_table_entry_destroy); - ctep_init = false; -} - -static struct counter_table_entry * -counter_table_entry_borrow(void) -{ - struct counter_table_entry *cte; - - FREEPOOL_BORROW(cte, &ctep, next, counter_table_entry_create); - if (cte == NULL) { - log_debug("borrow counter_table_entry failed: OOM"); - return NULL; - } - counter_table_entry_reset(cte); - - return cte; -} - -static void -counter_table_entry_return(struct counter_table_entry **counter_table_entry) -{ - struct counter_table_entry *cte = *counter_table_entry; - - if (cte == NULL) { - return; - } - - FREEPOOL_RETURN(cte, &ctep, next); - - *counter_table_entry = NULL; -} - -static void -counter_table_entry_pool_create(uint32_t max) -{ - struct counter_table_entry *cte; - - if (ctep_init) { - log_warn("counter_table_entry pool has already been created, re-creating"); - counter_table_entry_pool_destroy(); - } - - log_info("creating counter_table_entry pool: max %"PRIu32, max); - - FREEPOOL_CREATE(&ctep, max); - ctep_init = true; - - FREEPOOL_PREALLOC(cte, &ctep, max, next, counter_table_entry_create); - if (ctep.nfree < max) { - log_crit("cannot preallocate counter_table_entry pool, OOM. abort"); - exit(EXIT_FAILURE); - } -} - -void -counter_table_setup(uint32_t size, uint32_t poolsize) -{ - uint32_t i; - - log_info("Set up the %s module", COUNTER_TABLE_MODULE_NAME); - - if (counter_table_init) { - log_warn("%s has already been setup, ignore", COUNTER_TABLE_MODULE_NAME); - return; - } - - table = cc_alloc(sizeof(*table) * size); - table_size = size; - - if (table == NULL) { - log_crit("Could not allocate counter table for hotkey - OOM"); - exit(EXIT_FAILURE); - } - - for (i = 0; i < size; ++i) { - STAILQ_INIT(&table[i]); - } - - counter_table_entry_pool_create(poolsize); -} - -void -counter_table_teardown(void) -{ - log_info("Tear down the %s module", COUNTER_TABLE_MODULE_NAME); - - if (!counter_table_init) { - log_warn("%s was not setup", COUNTER_TABLE_MODULE_NAME); - } - - if (table != NULL) { - uint32_t i; - /* free all entries in table */ - for (i = 0; i < table_size; ++i) { - struct counter_table_entry *cte, *tcte; - STAILQ_FOREACH_SAFE(cte, &(table[i]), next, tcte) { - counter_table_entry_return(&cte); - } - } - } - - counter_table_entry_pool_destroy(); - counter_table_init = false; -} - -static inline struct cte_slh * -_get_bucket(const char *key, uint32_t nkey) -{ - return &(table[hash(key, nkey, 0) % table_size]); -} - -uint32_t -counter_table_incr(char *key, uint32_t nkey) -{ - struct counter_table_entry *cte; - struct cte_slh *bucket; - - ASSERT(nkey <= MAX_KEY_LEN); - - bucket = _get_bucket(key, nkey); - - /* iterate through bucket looking for item */ - for (cte = STAILQ_FIRST(bucket); cte != NULL; cte = STAILQ_NEXT(cte, next)) { - if ((nkey == cte->nkey) && cc_memcmp(key, cte->key, nkey) == 0) { - /* found item */ - return ++cte->count; - } - } - - /* not found, insert entry */ - cte = counter_table_entry_borrow(); - cc_memcpy(cte->key, key, nkey); - cte->nkey = nkey; - cte->count = 1; - STAILQ_INSERT_HEAD(bucket, cte, next); - - return 1; -} - -void -counter_table_decr(char *key, uint32_t nkey) -{ - struct counter_table_entry *cte, *prev; - struct cte_slh *bucket; - - ASSERT(nkey <= MAX_KEY_LEN); - - bucket = _get_bucket(key, nkey); - - /* iterate through bucket looking for item */ - for (prev = NULL, cte = STAILQ_FIRST(bucket); cte != NULL; - prev = cte, cte = STAILQ_NEXT(cte, next)) { - if ((nkey == cte->nkey) && cc_memcmp(key, cte->key, nkey) == 0) { - /* found item */ - if (--(cte->count) == 0) { - /* remove entry */ - if (prev == NULL) { - STAILQ_REMOVE_HEAD(bucket, next); - } else { - STAILQ_REMOVE_AFTER(bucket, prev, next); - } - } - - return; - } - } - - /* item not found, should never happen for hotkey detection scheme */ - ASSERT(false); -} diff --git a/src/hotkey/counter_table.h b/src/hotkey/counter_table.h deleted file mode 100644 index d084400ba..000000000 --- a/src/hotkey/counter_table.h +++ /dev/null @@ -1,20 +0,0 @@ -#pragma once - -#include - -/* - * The counter_table module provides a utility for counting the frequency at - * which keys appear. When a key is sampled, it should be incremented in the - * table after enqueueing it. When a key is dequeued, it should be decremented - * from the table. - */ - -/* setup/teardown module */ -void counter_table_setup(uint32_t size, uint32_t poolsize); -void counter_table_teardown(void); - -/* increment count and return count, insert into table if count == 0 */ -uint32_t counter_table_incr(char *key, uint32_t nkey); - -/* decrement count, remove from table if count == 0 after decr */ -void counter_table_decr(char *key, uint32_t nkey); diff --git a/src/hotkey/hotkey.c b/src/hotkey/hotkey.c index a925c0abb..b14ae51c4 100644 --- a/src/hotkey/hotkey.c +++ b/src/hotkey/hotkey.c @@ -1,9 +1,10 @@ #include "hotkey.h" #include "constant.h" -#include "counter_table.h" -#include "queue.h" +#include "kc_map.h" +#include "key_window.h" +#include #include #define HOTKEY_MODULE_NAME "hotkey::hotkey" @@ -13,10 +14,10 @@ bool hotkey_enabled = false; static uint64_t hotkey_counter; static bool hotkey_init = false; -static uint32_t hotkey_nsample = HOTKEY_NSAMPLE; +static uint32_t hotkey_window_size = HOTKEY_WINDOW_SIZE; static uint32_t hotkey_rate = HOTKEY_RATE; static uint32_t hotkey_threshold = HOTKEY_THRESHOLD; -static uint32_t hotkey_nsample_cur = 0; +static uint32_t hotkey_window_size_cur = 0; void hotkey_setup(hotkey_options_st *options) @@ -25,16 +26,16 @@ hotkey_setup(hotkey_options_st *options) if (options != NULL) { hotkey_enabled = option_bool(&options->hotkey_enable); - hotkey_nsample = option_uint(&options->hotkey_sample_size); + hotkey_window_size = option_uint(&options->hotkey_sample_size); hotkey_rate = option_uint(&options->hotkey_sample_rate); - hotkey_threshold = option_uint(&options->hotkey_threshold); + hotkey_threshold = (uint32_t)(option_fpn(&options->hotkey_threshold_ratio) * hotkey_window_size); } - hotkey_nsample_cur = 0; + hotkey_window_size_cur = 0; hotkey_counter = 0; - queue_setup(hotkey_nsample); + key_window_setup(hotkey_window_size); /* TODO: determine whether table size should be a tuneable parameter */ - counter_table_setup(hotkey_nsample, hotkey_nsample); + kc_map_setup(hotkey_window_size, hotkey_window_size); hotkey_init = true; } @@ -45,32 +46,35 @@ hotkey_teardown(void) if (!hotkey_init) { log_warn("%s was not setup", HOTKEY_MODULE_NAME); + return; } hotkey_enabled = false; - queue_teardown(); - counter_table_teardown(); + key_window_teardown(); + kc_map_teardown(); hotkey_init = false; } bool -_hotkey_sample(char *key, uint32_t nkey) +hotkey_sample(const struct bstring *key) { if (++hotkey_counter % hotkey_rate == 0) { /* sample this key */ uint32_t freq; - if (queue_len() == hotkey_nsample) { + if (key_window_len() == hotkey_window_size) { char buf[MAX_KEY_LEN]; - uint32_t len; + struct bstring popped; - /* pop from queue, decrement in counter table */ - len = queue_pop(buf); - counter_table_decr(buf, len); + popped.data = buf; + + /* pop from key_window, decrement in counter table */ + popped.len = key_window_pop(popped.data); + kc_map_decr(&popped); } - queue_push(key, nkey); - freq = counter_table_incr(key, nkey); + key_window_push(key); + freq = kc_map_incr(key); return freq >= hotkey_threshold; } diff --git a/src/hotkey/hotkey.h b/src/hotkey/hotkey.h index be3e39ed5..3b6350c76 100644 --- a/src/hotkey/hotkey.h +++ b/src/hotkey/hotkey.h @@ -7,16 +7,17 @@ /* TODO(kevyang): add stats for hotkey module */ -#define HOTKEY_NSAMPLE 10000 /* keep last 10000 keys sampled by default */ -#define HOTKEY_RATE 100 /* sample one in every 100 keys by default */ -#define HOTKEY_THRESHOLD 10 /* signal for hotkey if 10 or more keys in sample by default */ - -/* name type default description */ -#define HOTKEY_OPTION(ACTION) \ - ACTION( hotkey_enable, OPTION_TYPE_BOOL, false, "use hotkey detection?" )\ - ACTION( hotkey_sample_size, OPTION_TYPE_UINT, HOTKEY_NSAMPLE, "number of keys to maintain" )\ - ACTION( hotkey_sample_rate, OPTION_TYPE_UINT, HOTKEY_RATE, "hotkey sample ratio" )\ - ACTION( hotkey_threshold, OPTION_TYPE_UINT, HOTKEY_THRESHOLD, "threshold for hotkey signal") +#define HOTKEY_WINDOW_SIZE 10000 /* keep last 10000 keys sampled by default */ +#define HOTKEY_RATE 100 /* sample one in every 100 keys by default */ +#define HOTKEY_THRESHOLD_RATIO 0.01 /* signal hotkey if key takes up >= 0.01 of all keys by default */ +#define HOTKEY_THRESHOLD (uint32_t)(HOTKEY_THRESHOLD_RATIO * HOTKEY_WINDOW_SIZE) + +/* name type default description */ +#define HOTKEY_OPTION(ACTION) \ + ACTION( hotkey_enable, OPTION_TYPE_BOOL, false, "use hotkey detection?" )\ + ACTION( hotkey_sample_size, OPTION_TYPE_UINT, HOTKEY_WINDOW_SIZE, "number of keys to maintain" )\ + ACTION( hotkey_sample_rate, OPTION_TYPE_UINT, HOTKEY_RATE, "hotkey sample ratio" )\ + ACTION( hotkey_threshold_ratio, OPTION_TYPE_UINT, HOTKEY_THRESHOLD_RATIO, "threshold for hotkey signal") typedef struct { HOTKEY_OPTION(OPTION_DECLARE) @@ -26,11 +27,4 @@ extern bool hotkey_enabled; void hotkey_setup(hotkey_options_st *options); void hotkey_teardown(void); - -#define hotkey_sample(key, nkey) do { \ - if (hotkey_enabled) { \ - _hotkey_sample(key, nkey); \ - } \ -} while (0) - -bool _hotkey_sample(char *key, uint32_t nkey); +bool hotkey_sample(const struct bstring *key); diff --git a/src/hotkey/kc_map.c b/src/hotkey/kc_map.c new file mode 100644 index 000000000..b4b4d6356 --- /dev/null +++ b/src/hotkey/kc_map.c @@ -0,0 +1,244 @@ +#include "kc_map.h" + +#include "constant.h" + +#include +#include +#include +#include +#include + +#define KC_MAP_MODULE_NAME "hotkey::kc_map" + +struct kc_map_entry { + STAILQ_ENTRY(kc_map_entry) next; /* entry in hash table or pool */ + + char key[MAX_KEY_LEN]; + uint32_t klen; + uint32_t count; +}; + +STAILQ_HEAD(kcme_slh, kc_map_entry); + +static struct kcme_slh *table = NULL; +static uint32_t table_size = 0; /* number of buckets in table */ +static bool kc_map_init = false; + +FREEPOOL(kcme_pool, kcmeq, kc_map_entry); +static struct kcme_pool kcmep; +static bool kcmep_init = false; + +static void +kc_map_entry_reset(struct kc_map_entry *kcme) +{ + kcme->klen = 0; + kcme->count = 0; +} + +static struct kc_map_entry * +kc_map_entry_create(void) +{ + struct kc_map_entry *kcme = cc_alloc(sizeof(*kcme)); + + if (kcme == NULL) { + return NULL; + } + + kc_map_entry_reset(kcme); + + return kcme; +} + +static void +kc_map_entry_destroy(struct kc_map_entry **kc_map_entry) +{ + struct kc_map_entry *kcme = *kc_map_entry; + ASSERT(kcme != NULL); + + cc_free(kcme); + *kc_map_entry = NULL; +} + +static void +kc_map_entry_pool_destroy(void) +{ + struct kc_map_entry *kcme, *tkcme; + + if (!kcmep_init) { + log_warn("kc_map_entry pool was not created, ignore"); + return; + } + + log_info("destroying kc_map_entry pool: free %"PRIu32, kcmep.nfree); + + FREEPOOL_DESTROY(kcme, tkcme, &kcmep, next, kc_map_entry_destroy); + kcmep_init = false; +} + +static struct kc_map_entry * +kc_map_entry_borrow(void) +{ + struct kc_map_entry *kcme; + + FREEPOOL_BORROW(kcme, &kcmep, next, kc_map_entry_create); + if (kcme == NULL) { + log_debug("borrow kc_map_entry failed: OOM"); + return NULL; + } + kc_map_entry_reset(kcme); + + return kcme; +} + +static void +kc_map_entry_return(struct kc_map_entry **kc_map_entry) +{ + struct kc_map_entry *kcme = *kc_map_entry; + + if (kcme == NULL) { + return; + } + + FREEPOOL_RETURN(kcme, &kcmep, next); + + *kc_map_entry = NULL; +} + +static void +kc_map_entry_pool_create(uint32_t max) +{ + struct kc_map_entry *kcme; + + if (kcmep_init) { + log_warn("kc_map_entry pool has already been created, re-creating"); + kc_map_entry_pool_destroy(); + } + + log_info("creating kc_map_entry pool: max %"PRIu32, max); + + FREEPOOL_CREATE(&kcmep, max); + kcmep_init = true; + + FREEPOOL_PREALLOC(kcme, &kcmep, max, next, kc_map_entry_create); + if (kcmep.nfree < max) { + log_crit("cannot preallocate kc_map_entry pool, OOM. abort"); + exit(EXIT_FAILURE); + } +} + +void +kc_map_setup(uint32_t size, uint32_t poolsize) +{ + uint32_t i; + + log_info("Set up the %s module", KC_MAP_MODULE_NAME); + + if (kc_map_init) { + log_warn("%s has already been setup, ignore", KC_MAP_MODULE_NAME); + return; + } + + table = cc_alloc(sizeof(*table) * size); + table_size = size; + + if (table == NULL) { + log_crit("Could not allocate counter table for hotkey - OOM"); + exit(EXIT_FAILURE); + } + + for (i = 0; i < size; ++i) { + STAILQ_INIT(&table[i]); + } + + kc_map_entry_pool_create(poolsize); +} + +void +kc_map_teardown(void) +{ + log_info("Tear down the %s module", KC_MAP_MODULE_NAME); + + if (!kc_map_init) { + log_warn("%s was not setup", KC_MAP_MODULE_NAME); + } + + if (table != NULL) { + uint32_t i; + /* free all entries in table */ + for (i = 0; i < table_size; ++i) { + struct kc_map_entry *kcme, *tkcme; + STAILQ_FOREACH_SAFE(kcme, &(table[i]), next, tkcme) { + kc_map_entry_return(&kcme); + } + } + } + + kc_map_entry_pool_destroy(); + kc_map_init = false; +} + +static inline struct kcme_slh * +_get_bucket(const struct bstring *key) +{ + return &(table[hash(key->data, key->len, 0) % table_size]); +} + +uint32_t +kc_map_incr(const struct bstring *key) +{ + struct kc_map_entry *kcme; + struct kcme_slh *bucket; + + ASSERT(key->len <= MAX_KEY_LEN); + + bucket = _get_bucket(key); + + /* iterate through bucket looking for item */ + for (kcme = STAILQ_FIRST(bucket); kcme != NULL; kcme = STAILQ_NEXT(kcme, next)) { + if ((key->len == kcme->klen) && cc_memcmp(key->data, kcme->key, key->len) == 0) { + /* found item */ + return ++kcme->count; + } + } + + /* not found, insert entry */ + kcme = kc_map_entry_borrow(); + cc_memcpy(kcme->key, key->data, key->len); + kcme->klen = key->len; + kcme->count = 1; + STAILQ_INSERT_HEAD(bucket, kcme, next); + + return 1; +} + +void +kc_map_decr(const struct bstring *key) +{ + struct kc_map_entry *kcme, *prev; + struct kcme_slh *bucket; + + ASSERT(key->len <= MAX_KEY_LEN); + + bucket = _get_bucket(key); + + /* iterate through bucket looking for item */ + for (prev = NULL, kcme = STAILQ_FIRST(bucket); kcme != NULL; + prev = kcme, kcme = STAILQ_NEXT(kcme, next)) { + if ((key->len == kcme->klen) && cc_memcmp(key->data, kcme->key, key->len) == 0) { + /* found item */ + if (--(kcme->count) == 0) { + /* remove entry */ + if (prev == NULL) { + STAILQ_REMOVE_HEAD(bucket, next); + } else { + STAILQ_REMOVE_AFTER(bucket, prev, next); + } + } + + return; + } + } + + /* item not found, should never happen for hotkey detection scheme */ + ASSERT(false); +} diff --git a/src/hotkey/kc_map.h b/src/hotkey/kc_map.h new file mode 100644 index 000000000..a7228857c --- /dev/null +++ b/src/hotkey/kc_map.h @@ -0,0 +1,22 @@ +#pragma once + +#include + +/* + * The kc_map module provides a utility for counting the frequency at which keys + * appear. When a key is sampled, it should be incremented in the table after + * enqueueing it. When a key is dequeued, it should be decremented from the + * table. + */ + +struct bstring; + +/* setup/teardown module */ +void kc_map_setup(uint32_t size, uint32_t poolsize); +void kc_map_teardown(void); + +/* increment count and return count, insert into table if count == 0 */ +uint32_t kc_map_incr(const struct bstring *key); + +/* decrement count, remove from table if count == 0 after decr */ +void kc_map_decr(const struct bstring *key); diff --git a/src/hotkey/key_window.c b/src/hotkey/key_window.c new file mode 100644 index 000000000..3b19086a2 --- /dev/null +++ b/src/hotkey/key_window.c @@ -0,0 +1,195 @@ +#include "key_window.h" +#include "constant.h" + +#include +#include +#include +#include + +#define KEY_WINDOW_MODULE_NAME "hotkey::key_window" + +static bool key_window_init = false; +static int key_window_size = 0; + +FREEPOOL(kwn_pool, kwnq, key_window_node); +static struct kwn_pool kwnp; +static bool kwnp_init = false; + +struct key_window_node { + char key[MAX_KEY_LEN]; + uint32_t nkey; + STAILQ_ENTRY(key_window_node) next; +}; + +STAILQ_HEAD(key_window, key_window_node); +struct key_window q = STAILQ_HEAD_INITIALIZER(q); + +static void +key_window_node_reset(struct key_window_node *kwn) +{ + kwn->nkey = 0; +} + +static struct key_window_node * +key_window_node_create(void) +{ + struct key_window_node *kwn = cc_alloc(sizeof(*kwn)); + + if (kwn == NULL) { + return NULL; + } + + key_window_node_reset(kwn); + + return kwn; +} + +static void +key_window_node_destroy(struct key_window_node **key_window_node) +{ + struct key_window_node *kwn = *key_window_node; + ASSERT(kwn != NULL); + + cc_free(kwn); + *key_window_node = NULL; +} + +static void +key_window_node_pool_destroy(void) +{ + struct key_window_node *kwn, *tkwn; + + if (!kwnp_init) { + log_warn("key_window_node pool was not created, ignore"); + return; + } + + log_info("destroying key_window_node pool: free %"PRIu32, kwnp.nfree); + + FREEPOOL_DESTROY(kwn, tkwn, &kwnp, next, key_window_node_destroy); + kwnp_init = false; +} + +static void +key_window_node_pool_create(uint32_t max) +{ + struct key_window_node *kwn; + + if (kwnp_init) { + log_warn("key_window_node pool has already been created, re-creating"); + key_window_node_pool_destroy(); + } + + log_info("creating key_window_node pool: max %"PRIu32, max); + + FREEPOOL_CREATE(&kwnp, max); + kwnp_init = true; + + FREEPOOL_PREALLOC(kwn, &kwnp, max, next, key_window_node_create); + if (kwnp.nfree < max) { + log_crit("cannot preallocate key_window_node pool, OOM. abort"); + exit(EXIT_FAILURE); + } +} + +static struct key_window_node * +key_window_node_borrow(void) +{ + struct key_window_node *kwn; + + FREEPOOL_BORROW(kwn, &kwnp, next, key_window_node_create); + if (kwn == NULL) { + log_debug("borrow key_window_node failed: OOM"); + return NULL; + } + key_window_node_reset(kwn); + + return kwn; +} + +static void +key_window_node_return(struct key_window_node **key_window_node) +{ + struct key_window_node *kwn = *key_window_node; + + if (kwn == NULL) { + return; + } + + FREEPOOL_RETURN(kwn, &kwnp, next); + + *key_window_node = NULL; +} + +void +key_window_setup(uint32_t poolsize) +{ + log_info("set up the %s module", KEY_WINDOW_MODULE_NAME); + + if (key_window_init) { + log_warn("%s has already been setup, overwrite", KEY_WINDOW_MODULE_NAME); + } + + key_window_node_pool_create(poolsize); + key_window_size = 0; + STAILQ_INIT(&q); + key_window_init = true; +} + +void +key_window_teardown(void) +{ + struct key_window_node *kwn, *tkwn; + + log_info("tear down the %s module", KEY_WINDOW_MODULE_NAME); + + if (!key_window_init) { + log_warn("%s was not setup", KEY_WINDOW_MODULE_NAME); + return; + } + + /* free all entries in key_window */ + STAILQ_FOREACH_SAFE(kwn, &q, next, tkwn) { + key_window_node_return(&kwn); + } + + key_window_node_pool_destroy(); + key_window_init = false; +} + +void +key_window_push(const struct bstring *key) +{ + struct key_window_node *kwn = key_window_node_borrow(); + + ASSERT(key->len <= MAX_KEY_LEN); + + cc_memcpy(kwn->key, key->data, key->len); + kwn->nkey = key->len; + STAILQ_INSERT_TAIL(&q, kwn, next); + ++key_window_size; +} + +uint32_t +key_window_pop(char *buf) +{ + struct key_window_node *kwn = STAILQ_FIRST(&q); + uint32_t nkey; + + ASSERT(key_window_size > 0); + + cc_memcpy(buf, kwn->key, kwn->nkey); + nkey = kwn->nkey; + + STAILQ_REMOVE_HEAD(&q, next); + key_window_node_return(&kwn); + --key_window_size; + + return nkey; +} + +uint32_t +key_window_len(void) +{ + return key_window_size; +} diff --git a/src/hotkey/key_window.h b/src/hotkey/key_window.h new file mode 100644 index 000000000..44ec97f0c --- /dev/null +++ b/src/hotkey/key_window.h @@ -0,0 +1,18 @@ +#pragma once + +#include + +/* + * The key_window module provides a FIFO key_window interface to help with bookkeeping the + * last N keys provided for hotkey detection. + */ + +struct bstring; + +void key_window_push(const struct bstring *key); +uint32_t key_window_pop(char *buf); /* returns length of key */ +uint32_t key_window_len(void); + +/* Setup/teardown functions, must be called before module is used */ +void key_window_setup(uint32_t poolsize); +void key_window_teardown(void); diff --git a/src/hotkey/queue.c b/src/hotkey/queue.c deleted file mode 100644 index f40d0845d..000000000 --- a/src/hotkey/queue.c +++ /dev/null @@ -1,190 +0,0 @@ -#include "queue.h" -#include "constant.h" - -#include -#include -#include - -#define QUEUE_MODULE_NAME "hotkey::queue" - -static bool queue_init = false; -static int queue_size = 0; - -FREEPOOL(qn_pool, qnq, queue_node); -static struct qn_pool qnp; -static bool qnp_init = false; - -struct queue_node { - char key[MAX_KEY_LEN]; - uint32_t nkey; - STAILQ_ENTRY(queue_node) next; -}; - -STAILQ_HEAD(queue, queue_node); -struct queue q = STAILQ_HEAD_INITIALIZER(q); - -static void -queue_node_reset(struct queue_node *qn) -{ - qn->nkey = 0; -} - -static struct queue_node * -queue_node_create(void) -{ - struct queue_node *qn = cc_alloc(sizeof(*qn)); - - if (qn == NULL) { - return NULL; - } - - queue_node_reset(qn); - - return qn; -} - -static void -queue_node_destroy(struct queue_node **queue_node) -{ - struct queue_node *qn = *queue_node; - ASSERT(qn != NULL); - - cc_free(qn); - *queue_node = NULL; -} - -static void -queue_node_pool_destroy(void) -{ - struct queue_node *qn, *tqn; - - if (!qnp_init) { - log_warn("queue_node pool was not created, ignore"); - } - - log_info("destroying queue_node pool: free %"PRIu32, qnp.nfree); - - FREEPOOL_DESTROY(qn, tqn, &qnp, next, queue_node_destroy); - qnp_init = false; -} - -static void -queue_node_pool_create(uint32_t max) -{ - struct queue_node *qn; - - if (qnp_init) { - log_warn("queue_node pool has already been created, re-creating"); - queue_node_pool_destroy(); - } - - log_info("creating queue_node pool: max %"PRIu32, max); - - FREEPOOL_CREATE(&qnp, max); - qnp_init = true; - - FREEPOOL_PREALLOC(qn, &qnp, max, next, queue_node_create); - if (qnp.nfree < max) { - log_crit("cannot preallocate queue_node pool, OOM. abort"); - exit(EXIT_FAILURE); - } -} - -static struct queue_node * -queue_node_borrow(void) -{ - struct queue_node *qn; - - FREEPOOL_BORROW(qn, &qnp, next, queue_node_create); - if (qn == NULL) { - log_debug("borrow queue_node failed: OOM"); - return NULL; - } - queue_node_reset(qn); - - return qn; -} - -static void -queue_node_return(struct queue_node **queue_node) -{ - struct queue_node *qn = *queue_node; - - if (qn == NULL) { - return; - } - - FREEPOOL_RETURN(qn, &qnp, next); - - *queue_node = NULL; -} - -void -queue_setup(uint32_t poolsize) -{ - log_info("set up the %s module", QUEUE_MODULE_NAME); - - if (queue_init) { - log_warn("%s has already been setup, overwrite", QUEUE_MODULE_NAME); - } - - queue_node_pool_create(poolsize); - queue_size = 0; - STAILQ_INIT(&q); - queue_init = true; -} - -void -queue_teardown(void) -{ - struct queue_node *qn, *tqn; - - log_info("tear down the %s module", QUEUE_MODULE_NAME); - - if (!queue_init) { - log_warn("%s was not setup", QUEUE_MODULE_NAME); - } - - /* free all entries in queue */ - STAILQ_FOREACH_SAFE(qn, &q, next, tqn) { - queue_node_return(&qn); - } - - queue_node_pool_destroy(); - queue_init = false; -} - -void -queue_push(char *key, uint32_t nkey) -{ - struct queue_node *qn = queue_node_borrow(); - - ASSERT(nkey <= MAX_KEY_LEN); - - cc_memcpy(qn->key, key, nkey); - qn->nkey = nkey; - STAILQ_INSERT_TAIL(&q, qn, next); - ++queue_size; -} - -uint32_t -queue_pop(char *buf) -{ - struct queue_node *qn = STAILQ_FIRST(&q); - uint32_t nkey; - - cc_memcpy(buf, qn->key, qn->nkey); - nkey = qn->nkey; - - STAILQ_REMOVE_HEAD(&q, next); - queue_node_return(&qn); - --queue_size; - - return nkey; -} - -uint32_t -queue_len(void) -{ - return queue_size; -} diff --git a/src/hotkey/queue.h b/src/hotkey/queue.h deleted file mode 100644 index d165f6796..000000000 --- a/src/hotkey/queue.h +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include - -/* - * The queue module provides a FIFO queue interface to help with bookkeeping the - * last N keys provided for hotkey detection. - */ - -void queue_push(char *key, uint32_t nkey); -uint32_t queue_pop(char *buf); /* returns length of key */ -uint32_t queue_len(void); - -/* Setup/teardown functions, must be called before module is used */ -void queue_setup(uint32_t poolsize); -void queue_teardown(void); diff --git a/test/hotkey/#check_queue.c# b/test/hotkey/#check_queue.c# deleted file mode 100644 index e69de29bb..000000000 diff --git a/test/hotkey/.#check_queue.c b/test/hotkey/.#check_queue.c deleted file mode 120000 index 628b1e49b..000000000 --- a/test/hotkey/.#check_queue.c +++ /dev/null @@ -1 +0,0 @@ -kyang@tw-mbp-kyang.67519 \ No newline at end of file diff --git a/test/hotkey/CMakeLists.txt b/test/hotkey/CMakeLists.txt index 9cc2a02ca..109a0e1c2 100644 --- a/test/hotkey/CMakeLists.txt +++ b/test/hotkey/CMakeLists.txt @@ -1,2 +1,2 @@ -add_subdirectory(counter_table) -add_subdirectory(queue) +add_subdirectory(kc_map) +add_subdirectory(key_window) diff --git a/test/hotkey/queue/CMakeLists.txt b/test/hotkey/kc_map/CMakeLists.txt similarity index 94% rename from test/hotkey/queue/CMakeLists.txt rename to test/hotkey/kc_map/CMakeLists.txt index b1402543e..505070af4 100644 --- a/test/hotkey/queue/CMakeLists.txt +++ b/test/hotkey/kc_map/CMakeLists.txt @@ -1,4 +1,4 @@ -set(suite queue) +set(suite kc_map) set(test_name check_${suite}) set(source check_${suite}.c) diff --git a/test/hotkey/counter_table/check_counter_table.c b/test/hotkey/kc_map/check_kc_map.c similarity index 60% rename from test/hotkey/counter_table/check_counter_table.c rename to test/hotkey/kc_map/check_kc_map.c index d27b9f5a5..07a4261a5 100644 --- a/test/hotkey/counter_table/check_counter_table.c +++ b/test/hotkey/kc_map/check_kc_map.c @@ -1,12 +1,15 @@ -#include +#include #include #include +#include + #include +#include -#define SUITE_NAME "counter_table" +#define SUITE_NAME "kc_map" #define DEBUG_LOG SUITE_NAME ".log" #define TEST_TABLE_SIZE 10 @@ -17,13 +20,13 @@ static void test_setup(void) { - counter_table_setup(TEST_TABLE_SIZE, TEST_TABLE_SIZE); + kc_map_setup(TEST_TABLE_SIZE, TEST_TABLE_SIZE); } static void test_teardown(void) { - counter_table_teardown(); + kc_map_teardown(); } static void @@ -39,23 +42,28 @@ test_reset(void) START_TEST(test_basic) { - char *key1 = "key1", *key2 = "key2"; - uint32_t klen = 4; + char *key1str = "key1", *key2str = "key22"; uint32_t count; + struct bstring key1, key2; + + key1.data = key1str; + key1.len = strlen(key1str); + key2.data = key2str; + key2.len = strlen(key2str);; test_reset(); - count = counter_table_incr(key1, klen); + count = kc_map_incr(&key1); ck_assert_int_eq(count, 1); - count = counter_table_incr(key2, klen); + count = kc_map_incr(&key2); ck_assert_int_eq(count, 1); - count = counter_table_incr(key1, klen); + count = kc_map_incr(&key1); ck_assert_int_eq(count, 2); - counter_table_decr(key1, klen); - count = counter_table_incr(key1, klen); + kc_map_decr(&key1); + count = kc_map_incr(&key1); ck_assert_int_eq(count, 2); } END_TEST @@ -64,15 +72,15 @@ END_TEST * test suite */ static Suite * -counter_table_suite(void) +kc_map_suite(void) { Suite *s = suite_create(SUITE_NAME); /* basic functionality */ - TCase *tc_basic_counter_table = tcase_create("basic counter_table"); - suite_add_tcase(s, tc_basic_counter_table); + TCase *tc_basic_kc_map = tcase_create("basic kc_map"); + suite_add_tcase(s, tc_basic_kc_map); - tcase_add_test(tc_basic_counter_table, test_basic); + tcase_add_test(tc_basic_kc_map, test_basic); return s; } @@ -85,7 +93,7 @@ main(void) /* setup */ test_setup(); - Suite *suite = counter_table_suite(); + Suite *suite = kc_map_suite(); SRunner *srunner = srunner_create(suite); srunner_set_log(srunner, DEBUG_LOG); srunner_run_all(srunner, CK_ENV); /* set CK_VERBOSITY in ENV to customize */ diff --git a/test/hotkey/counter_table/CMakeLists.txt b/test/hotkey/key_window/CMakeLists.txt similarity index 91% rename from test/hotkey/counter_table/CMakeLists.txt rename to test/hotkey/key_window/CMakeLists.txt index d4c0802d1..cd4b688e4 100644 --- a/test/hotkey/counter_table/CMakeLists.txt +++ b/test/hotkey/key_window/CMakeLists.txt @@ -1,4 +1,4 @@ -set(suite counter_table) +set(suite key_window) set(test_name check_${suite}) set(source check_${suite}.c) diff --git a/test/hotkey/key_window/check_key_window.c b/test/hotkey/key_window/check_key_window.c new file mode 100644 index 000000000..c661e4d6b --- /dev/null +++ b/test/hotkey/key_window/check_key_window.c @@ -0,0 +1,145 @@ +#include + +#include + +#include + +#include + +#include +#include +#include + +#define SUITE_NAME "key_window" +#define DEBUG_LOG SUITE_NAME ".log" + +#define TEST_KEY_WINDOW_SIZE 10 + +/* + * utilities + */ +static void +test_setup(void) +{ + key_window_setup(TEST_KEY_WINDOW_SIZE); +} + +static void +test_teardown(void) +{ + key_window_teardown(); +} + +static void +test_reset(void) +{ + test_teardown(); + test_setup(); +} + +/************** + * test cases * + **************/ + +START_TEST(test_basic) +{ + char *key1str = "key1"; + char buf[MAX_KEY_LEN]; + uint32_t key_window_pop_len; + struct bstring key1; + + key1.data = key1str; + key1.len = strlen(key1str); + + test_reset(); + + ck_assert_int_eq(key_window_len(), 0); + + key_window_push(&key1); + ck_assert_int_eq(key_window_len(), 1); + key_window_pop_len = key_window_pop(buf); + + ck_assert_int_eq(key_window_len(), 0); + ck_assert_int_eq(key_window_pop_len, key1.len); + ck_assert(strncmp(key1.data, buf, key1.len) == 0); +} +END_TEST + +START_TEST(test_multiple) +{ + char *key1str = "key1", *key2str = "key22", *key3str = "key333"; + char buf[MAX_KEY_LEN]; + uint32_t key_window_pop_len; + struct bstring key1, key2, key3; + + key1.data = key1str; + key1.len = strlen(key1str); + key2.data = key2str; + key2.len = strlen(key2str); + key3.data = key3str; + key3.len = strlen(key3str); + + test_reset(); + + ck_assert_int_eq(key_window_len(), 0); + + key_window_push(&key1); + key_window_push(&key2); + key_window_push(&key3); + ck_assert_int_eq(key_window_len(), 3); + + key_window_pop_len = key_window_pop(buf); + ck_assert_int_eq(key_window_pop_len, key1.len); + ck_assert(strncmp(key1.data, buf, key1.len) == 0); + ck_assert_int_eq(key_window_len(), 2); + + key_window_pop_len = key_window_pop(buf); + ck_assert_int_eq(key_window_pop_len, key2.len); + ck_assert(strncmp(key2.data, buf, key2.len) == 0); + ck_assert_int_eq(key_window_len(), 1); + + key_window_pop_len = key_window_pop(buf); + ck_assert_int_eq(key_window_pop_len, key3.len); + ck_assert(strncmp(key3.data, buf, key3.len) == 0); + ck_assert_int_eq(key_window_len(), 0); +} +END_TEST + +/* + * test suite + */ +static Suite * +key_window_suite(void) +{ + Suite *s = suite_create(SUITE_NAME); + + /* basic key_window functionality */ + TCase *tc_basic_key_window = tcase_create("basic key_window"); + suite_add_tcase(s, tc_basic_key_window); + + tcase_add_test(tc_basic_key_window, test_basic); + tcase_add_test(tc_basic_key_window, test_multiple); + + return s; +} + +int +main(void) +{ + int nfail; + + /* setup */ + test_setup(); + + Suite *suite = key_window_suite(); + SRunner *srunner = srunner_create(suite); + srunner_set_log(srunner, DEBUG_LOG); + srunner_run_all(srunner, CK_ENV); /* set CK_VERBOSITY in ENV to customize */ + nfail = srunner_ntests_failed(srunner); + srunner_free(srunner); + + /* teardown */ + test_teardown(); + + return (nfail == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} diff --git a/test/hotkey/queue/check_queue.c b/test/hotkey/queue/check_queue.c deleted file mode 100644 index c2e95d80e..000000000 --- a/test/hotkey/queue/check_queue.c +++ /dev/null @@ -1,133 +0,0 @@ -#include - -#include - -#include - -#include -#include -#include - -#define SUITE_NAME "queue" -#define DEBUG_LOG SUITE_NAME ".log" - -#define TEST_QUEUE_SIZE 10 - -/* - * utilities - */ -static void -test_setup(void) -{ - queue_setup(TEST_QUEUE_SIZE); -} - -static void -test_teardown(void) -{ - queue_teardown(); -} - -static void -test_reset(void) -{ - test_teardown(); - test_setup(); -} - -/************** - * test cases * - **************/ - -START_TEST(test_basic) -{ - char *key = "key1"; - uint32_t klen = 4; - char buf[MAX_KEY_LEN]; - uint32_t queue_pop_len; - - test_reset(); - - ck_assert_int_eq(queue_len(), 0); - - queue_push(key, klen); - ck_assert_int_eq(queue_len(), 1); - queue_pop_len = queue_pop(buf); - - ck_assert_int_eq(queue_len(), 0); - ck_assert_int_eq(queue_pop_len, klen); - ck_assert(strncmp(key, buf, klen) == 0); -} -END_TEST - -START_TEST(test_multiple) -{ - char *key1 = "key1", *key2 = "key22", *key3 = "key333"; - uint32_t klen1 = 4, klen2 = 5, klen3 = 6; - char buf[MAX_KEY_LEN]; - uint32_t queue_pop_len; - - test_reset(); - - ck_assert_int_eq(queue_len(), 0); - - queue_push(key1, klen1); - queue_push(key2, klen2); - queue_push(key3, klen3); - ck_assert_int_eq(queue_len(), 3); - - queue_pop_len = queue_pop(buf); - ck_assert_int_eq(queue_pop_len, klen1); - ck_assert(strncmp(key1, buf, klen1) == 0); - ck_assert_int_eq(queue_len(), 2); - - queue_pop_len = queue_pop(buf); - ck_assert_int_eq(queue_pop_len, klen2); - ck_assert(strncmp(key2, buf, klen2) == 0); - ck_assert_int_eq(queue_len(), 1); - - queue_pop_len = queue_pop(buf); - ck_assert_int_eq(queue_pop_len, klen3); - ck_assert(strncmp(key3, buf, klen3) == 0); - ck_assert_int_eq(queue_len(), 0); -} -END_TEST - -/* - * test suite - */ -static Suite * -queue_suite(void) -{ - Suite *s = suite_create(SUITE_NAME); - - /* basic queue functionality */ - TCase *tc_basic_queue = tcase_create("basic queue"); - suite_add_tcase(s, tc_basic_queue); - - tcase_add_test(tc_basic_queue, test_basic); - tcase_add_test(tc_basic_queue, test_multiple); - - return s; -} - -int -main(void) -{ - int nfail; - - /* setup */ - test_setup(); - - Suite *suite = queue_suite(); - SRunner *srunner = srunner_create(suite); - srunner_set_log(srunner, DEBUG_LOG); - srunner_run_all(srunner, CK_ENV); /* set CK_VERBOSITY in ENV to customize */ - nfail = srunner_ntests_failed(srunner); - srunner_free(srunner); - - /* teardown */ - test_teardown(); - - return (nfail == 0) ? EXIT_SUCCESS : EXIT_FAILURE; -} From 4d15b8b4a34af0aaf8f43455553d1afc365253eb Mon Sep 17 00:00:00 2001 From: Kevin Yang Date: Wed, 22 Feb 2017 13:50:12 -0800 Subject: [PATCH 3/3] fix test keys --- test/hotkey/kc_map/check_kc_map.c | 12 +++++------ test/hotkey/key_window/check_key_window.c | 25 ++++++++++------------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/test/hotkey/kc_map/check_kc_map.c b/test/hotkey/kc_map/check_kc_map.c index 07a4261a5..f384e9cb1 100644 --- a/test/hotkey/kc_map/check_kc_map.c +++ b/test/hotkey/kc_map/check_kc_map.c @@ -42,14 +42,10 @@ test_reset(void) START_TEST(test_basic) { - char *key1str = "key1", *key2str = "key22"; +#define KEY1 "key1" +#define KEY2 "key22" uint32_t count; - struct bstring key1, key2; - - key1.data = key1str; - key1.len = strlen(key1str); - key2.data = key2str; - key2.len = strlen(key2str);; + struct bstring key1 = str2bstr(KEY1), key2 = str2bstr(KEY2); test_reset(); @@ -65,6 +61,8 @@ START_TEST(test_basic) kc_map_decr(&key1); count = kc_map_incr(&key1); ck_assert_int_eq(count, 2); +#undef KEY1 +#undef KEY2 } END_TEST diff --git a/test/hotkey/key_window/check_key_window.c b/test/hotkey/key_window/check_key_window.c index c661e4d6b..3929ae5d6 100644 --- a/test/hotkey/key_window/check_key_window.c +++ b/test/hotkey/key_window/check_key_window.c @@ -43,13 +43,10 @@ test_reset(void) START_TEST(test_basic) { - char *key1str = "key1"; +#define KEY1 "key1" + struct bstring key1 = str2bstr(KEY1); char buf[MAX_KEY_LEN]; uint32_t key_window_pop_len; - struct bstring key1; - - key1.data = key1str; - key1.len = strlen(key1str); test_reset(); @@ -62,22 +59,19 @@ START_TEST(test_basic) ck_assert_int_eq(key_window_len(), 0); ck_assert_int_eq(key_window_pop_len, key1.len); ck_assert(strncmp(key1.data, buf, key1.len) == 0); +#undef KEY1 } END_TEST START_TEST(test_multiple) { - char *key1str = "key1", *key2str = "key22", *key3str = "key333"; +#define KEY1 "key1" +#define KEY2 "key22" +#define KEY3 "key333" char buf[MAX_KEY_LEN]; uint32_t key_window_pop_len; - struct bstring key1, key2, key3; - - key1.data = key1str; - key1.len = strlen(key1str); - key2.data = key2str; - key2.len = strlen(key2str); - key3.data = key3str; - key3.len = strlen(key3str); + struct bstring key1 = str2bstr(KEY1), key2 = str2bstr(KEY2), + key3 = str2bstr(KEY3); test_reset(); @@ -102,6 +96,9 @@ START_TEST(test_multiple) ck_assert_int_eq(key_window_pop_len, key3.len); ck_assert(strncmp(key3.data, buf, key3.len) == 0); ck_assert_int_eq(key_window_len(), 0); +#undef KEY1 +#undef KEY2 +#undef KEY3 } END_TEST