core: cache.c cleanups, use queue.h freelist

cache constructors/destructors were never used, which just ended up
being wasted branches. Since there's no constructor/destructor for cache
objects, we can use the memory itself for the freelist.

This removes a doubling realloc for the freelist of cache objects and
simplifies the code a bunch.

4 files changed, 31 insertions, 145 deletions

diff --git a/cache.c b/cache.c
index 95548e7..0636060 100644
--- a/cache.c
+++ b/cache.c
@@ -2,6 +2,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <inttypes.h>
+#include <assert.h>
 
 #ifndef NDEBUG
 #include <signal.h>
@@ -10,37 +11,29 @@
 #include "cache.h"
 
 #ifndef NDEBUG
-const uint64_t redzone_pattern = 0xdeadbeefcafebabe;
+const uint64_t redzone_pattern = 0xdeadbeefcafedeed;
 int cache_error = 0;
 #endif
 
-const int initial_pool_size = 64;
-
-cache_t* cache_create(const char *name, size_t bufsize, size_t align,
-                      cache_constructor_t* constructor,
-                      cache_destructor_t* destructor) {
+cache_t* cache_create(const char *name, size_t bufsize, size_t align) {
     cache_t* ret = calloc(1, sizeof(cache_t));
     char* nm = strdup(name);
-    void** ptr = calloc(initial_pool_size, sizeof(void*));
-    if (ret == NULL || nm == NULL || ptr == NULL ||
+    if (ret == NULL || nm == NULL ||
         pthread_mutex_init(&ret->mutex, NULL) == -1) {
         free(ret);
         free(nm);
-        free(ptr);
         return NULL;
     }
 
     ret->name = nm;
-    ret->ptr = ptr;
-    ret->freetotal = initial_pool_size;
-    ret->constructor = constructor;
-    ret->destructor = destructor;
+    STAILQ_INIT(&ret->head);
 
 #ifndef NDEBUG
     ret->bufsize = bufsize + 2 * sizeof(redzone_pattern);
 #else
     ret->bufsize = bufsize;
 #endif
+    assert(ret->bufsize >= sizeof(struct cache_free_s));
 
     return ret;
 }
@@ -61,15 +54,12 @@ static inline void* get_object(void *ptr) {
 }
 
 void cache_destroy(cache_t *cache) {
-    while (cache->freecurr > 0) {
-        void *ptr = cache->ptr[--cache->freecurr];
-        if (cache->destructor) {
-            cache->destructor(get_object(ptr), NULL);
-        }
-        free(ptr);
+    while (!STAILQ_EMPTY(&cache->head)) {
+        struct cache_free_s *o = STAILQ_FIRST(&cache->head);
+        STAILQ_REMOVE_HEAD(&cache->head, c_next);
+        free(o);
     }
     free(cache->name);
-    free(cache->ptr);
     pthread_mutex_destroy(&cache->mutex);
     free(cache);
 }
@@ -86,18 +76,15 @@ void* do_cache_alloc(cache_t *cache) {
     void *ret;
     void *object;
     if (cache->freecurr > 0) {
-        ret = cache->ptr[--cache->freecurr];
+        ret = STAILQ_FIRST(&cache->head);
+        STAILQ_REMOVE_HEAD(&cache->head, c_next);
         object = get_object(ret);
+        cache->freecurr--;
     } else if (cache->limit == 0 || cache->total < cache->limit) {
         object = ret = malloc(cache->bufsize);
         if (ret != NULL) {
             object = get_object(ret);
 
-            if (cache->constructor != NULL &&
-                cache->constructor(object, NULL, 0) != 0) {
-                free(ret);
-                object = NULL;
-            }
             cache->total++;
         }
     } else {
@@ -143,29 +130,11 @@ void do_cache_free(cache_t *cache, void *ptr) {
     ptr = pre;
 #endif
     if (cache->limit != 0 && cache->limit < cache->total) {
-        /* Allow freeing in case the limit was revised downward */
-        if (cache->destructor) {
-            cache->destructor(ptr, NULL);
-        }
         free(ptr);
         cache->total--;
-    } else if (cache->freecurr < cache->freetotal) {
-        cache->ptr[cache->freecurr++] = ptr;
     } else {
-        /* try to enlarge free connections array */
-        size_t newtotal = cache->freetotal * 2;
-        void **new_free = realloc(cache->ptr, sizeof(char *) * newtotal);
-        if (new_free) {
-            cache->freetotal = newtotal;
-            cache->ptr = new_free;
-            cache->ptr[cache->freecurr++] = ptr;
-        } else {
-            if (cache->destructor) {
-                cache->destructor(ptr, NULL);
-            }
-            free(ptr);
-            cache->total--;
-        }
+        STAILQ_INSERT_TAIL(&cache->head, (struct cache_free_s *)ptr, c_next);
+        cache->freecurr++;
     }
 }
 
diff --git a/cache.h b/cache.h
index 43267e8..0e9be6e 100644
--- a/cache.h
+++ b/cache.h
@@ -2,31 +2,18 @@
 #ifndef CACHE_H
 #define CACHE_H
 #include <pthread.h>
+#include "queue.h"
 
 #ifndef NDEBUG
 /* may be used for debug purposes */
 extern int cache_error;
 #endif
 
-/**
- * Constructor used to initialize allocated objects
- *
- * @param obj pointer to the object to initialized.
- * @param notused1 This parameter is currently not used.
- * @param notused2 This parameter is currently not used.
- * @return you should return 0, but currently this is not checked
- */
-typedef int cache_constructor_t(void* obj, void* notused1, int notused2);
-/**
- * Destructor used to clean up allocated objects before they are
- * returned to the operating system.
- *
- * @param obj pointer to the object to clean up.
- * @param notused This parameter is currently not used.
- * @return you should return 0, but currently this is not checked
- */
-typedef void cache_destructor_t(void* obj, void* notused);
+struct cache_free_s {
+    STAILQ_ENTRY(cache_free_s) c_next;
+};
 
+//typedef STAILQ_HEAD(cache_head_s, cache_free_s) cache_head_t;
 /**
  * Definition of the structure to keep track of the internal details of
  * the cache allocator. Touching any of these variables results in
@@ -37,8 +24,8 @@ typedef struct {
     pthread_mutex_t mutex;
     /** Name of the cache objects in this cache (provided by the caller) */
     char *name;
-    /** List of pointers to available buffers in this cache */
-    void **ptr;
+    /** freelist of available buffers */
+    STAILQ_HEAD(cache_head, cache_free_s) head;
     /** The size of each element in this cache */
     size_t bufsize;
     /** The capacity of the list of elements */
@@ -49,10 +36,6 @@ typedef struct {
     int freecurr;
     /** A limit on the total number of elements */
     int limit;
-    /** The constructor to be called each time we allocate more memory */
-    cache_constructor_t* constructor;
-    /** The destructor to be called each time before we release memory */
-    cache_destructor_t* destructor;
 } cache_t;
 
 /**
@@ -73,9 +56,8 @@ typedef struct {
  *                   to the os.
  * @return a handle to an object cache if successful, NULL otherwise.
  */
-cache_t* cache_create(const char* name, size_t bufsize, size_t align,
-                      cache_constructor_t* constructor,
-                      cache_destructor_t* destructor);
+cache_t* cache_create(const char* name, size_t bufsize, size_t align);
+
 /**
  * Destroy an object cache.
  *
diff --git a/testapp.c b/testapp.c
index 390f175..2af409b 100644
--- a/testapp.c
+++ b/testapp.c
@@ -97,75 +97,16 @@ static void close_conn() {
 
 static enum test_return cache_create_test(void)
 {
-    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*),
-                                  NULL, NULL);
+    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*));
     assert(cache != NULL);
     cache_destroy(cache);
     return TEST_PASS;
 }
 
-const uint64_t constructor_pattern = 0xdeadcafebabebeef;
-
-static int cache_constructor(void *buffer, void *notused1, int notused2) {
-    uint64_t *ptr = buffer;
-    *ptr = constructor_pattern;
-    return 0;
-}
-
-static enum test_return cache_constructor_test(void)
-{
-    cache_t *cache = cache_create("test", sizeof(uint64_t), sizeof(uint64_t),
-                                  cache_constructor, NULL);
-    assert(cache != NULL);
-    uint64_t *ptr = cache_alloc(cache);
-    uint64_t pattern = *ptr;
-    cache_free(cache, ptr);
-    cache_destroy(cache);
-    return (pattern == constructor_pattern) ? TEST_PASS : TEST_FAIL;
-}
-
-static int cache_fail_constructor(void *buffer, void *notused1, int notused2) {
-    return 1;
-}
-
-static enum test_return cache_fail_constructor_test(void)
-{
-    enum test_return ret = TEST_PASS;
-
-    cache_t *cache = cache_create("test", sizeof(uint64_t), sizeof(uint64_t),
-                                  cache_fail_constructor, NULL);
-    assert(cache != NULL);
-    uint64_t *ptr = cache_alloc(cache);
-    if (ptr != NULL) {
-        ret = TEST_FAIL;
-    }
-    cache_destroy(cache);
-    return ret;
-}
-
-static void *destruct_data = 0;
-
-static void cache_destructor(void *buffer, void *notused) {
-    destruct_data = buffer;
-}
-
-static enum test_return cache_destructor_test(void)
-{
-    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*),
-                                  NULL, cache_destructor);
-    assert(cache != NULL);
-    char *ptr = cache_alloc(cache);
-    cache_free(cache, ptr);
-    cache_destroy(cache);
-
-    return (ptr == destruct_data) ? TEST_PASS : TEST_FAIL;
-}
-
 static enum test_return cache_reuse_test(void)
 {
     int ii;
-    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*),
-                                  NULL, NULL);
+    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*));
     if (cache == NULL) {
         return TEST_FAIL;
     }
@@ -183,8 +124,7 @@ static enum test_return cache_reuse_test(void)
 
 static enum test_return cache_bulkalloc(size_t datasize)
 {
-    cache_t *cache = cache_create("test", datasize, sizeof(char*),
-                                  NULL, NULL);
+    cache_t *cache = cache_create("test", datasize, sizeof(char*));
     if (cache == NULL) {
         return TEST_FAIL;
     }
@@ -219,8 +159,7 @@ static enum test_return test_issue_161(void)
 static enum test_return cache_redzone_test(void)
 {
 #ifndef HAVE_UMEM_H
-    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*),
-                                  NULL, NULL);
+    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*));
 
     if (cache == NULL) {
         return TEST_FAIL;
@@ -259,8 +198,7 @@ static enum test_return cache_limit_revised_downward_test(void)
     int limit = 10, allocated_num = limit + 1, i;
     char ** alloc_objs = calloc(allocated_num, sizeof(char *));
 
-    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*),
-                                  NULL, NULL);
+    cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*));
     assert(cache != NULL);
 
     /* cache->limit is 0 and we can allocate limit+1 items */
@@ -2307,9 +2245,6 @@ struct testcase {
 
 struct testcase testcases[] = {
     { "cache_create", cache_create_test },
-    { "cache_constructor", cache_constructor_test },
-    { "cache_constructor_fail", cache_fail_constructor_test },
-    { "cache_destructor", cache_destructor_test },
     { "cache_reuse", cache_reuse_test },
     { "cache_redzone", cache_redzone_test },
     { "cache_limit_revised_downward", cache_limit_revised_downward_test },
diff --git a/thread.c b/thread.c
index e4b1da7..874cc9c 100644
--- a/thread.c
+++ b/thread.c
@@ -424,7 +424,7 @@ static void setup_thread(LIBEVENT_THREAD *me) {
         exit(EXIT_FAILURE);
     }
 
-    me->rbuf_cache = cache_create("rbuf", READ_BUFFER_SIZE, sizeof(char *), NULL, NULL);
+    me->rbuf_cache = cache_create("rbuf", READ_BUFFER_SIZE, sizeof(char *));
     if (me->rbuf_cache == NULL) {
         fprintf(stderr, "Failed to create read buffer cache\n");
         exit(EXIT_FAILURE);
@@ -441,7 +441,7 @@ static void setup_thread(LIBEVENT_THREAD *me) {
         cache_set_limit(me->rbuf_cache, limit);
     }
 
-    me->io_cache = cache_create("io", sizeof(io_pending_t), sizeof(char*), NULL, NULL);
+    me->io_cache = cache_create("io", sizeof(io_pending_t), sizeof(char*));
     if (me->io_cache == NULL) {
         fprintf(stderr, "Failed to create IO object cache\n");
         exit(EXIT_FAILURE);