On Fri, 2010-01-08 at 11:15 +1100, Bojan Smojver wrote:
> Why can't we just use malloc() for this and hang a cleanup off the
> pool? It would use less memory anyway than having another pool for
> this.

Quick and dirty example of what I mean here.

-- 
Bojan
Index: tables/apr_hash.c
===================================================================
--- tables/apr_hash.c	(revision 897059)
+++ tables/apr_hash.c	(working copy)
@@ -69,13 +69,9 @@
  * index rather than the size so that we can use bitwise-AND for
  * modular arithmetic. The count of hash entries may be greater
  * depending on the chosen collision rate.
- *
- * We allocate the bucket array in a sub-pool, "array_pool". This allows us
- * to reclaim the old bucket array after an expansion.
  */
 struct apr_hash_t {
     apr_pool_t          *pool;
-    apr_pool_t          *array_pool;
     apr_hash_entry_t   **array;
     apr_hash_index_t     iterator;  /* For apr_hash_first(NULL, ...) */
     unsigned int         count, max;
@@ -90,22 +86,29 @@
  * Hash creation functions.
  */
 
+static apr_status_t array_cleanup(void *data)
+{
+    free(data);
+    return APR_SUCCESS;
+}
+
 static apr_hash_entry_t **alloc_array(apr_hash_t *ht, unsigned int max)
 {
-   return apr_pcalloc(ht->array_pool, sizeof(*ht->array) * (max + 1));
+   void *mem;
+
+   mem = calloc(max + 1, sizeof(*ht->array));
+   apr_pool_cleanup_register(ht->pool, mem,
+                             array_cleanup,apr_pool_cleanup_null);
+
+   return mem;
 }
 
 APR_DECLARE(apr_hash_t *) apr_hash_make(apr_pool_t *pool)
 {
-    apr_pool_t *array_pool;
     apr_hash_t *ht;
 
-    if (apr_pool_create(&array_pool, pool) != APR_SUCCESS)
-        return NULL;
-
     ht = apr_palloc(pool, sizeof(apr_hash_t));
     ht->pool = pool;
-    ht->array_pool = array_pool;
     ht->free = NULL;
     ht->count = 0;
     ht->max = INITIAL_MAX;
@@ -172,17 +175,10 @@
 
 static void expand_array(apr_hash_t *ht)
 {
-    apr_pool_t *new_array_pool;
-    apr_pool_t *old_array_pool;
     apr_hash_index_t *hi;
     apr_hash_entry_t **new_array;
     unsigned int new_max;
 
-    if (apr_pool_create(&new_array_pool, ht->pool) != APR_SUCCESS)
-        return; /* Give up and don't try to expand the array */
-    old_array_pool = ht->array_pool;
-    ht->array_pool = new_array_pool;
-
     new_max = ht->max * 2 + 1;
     new_array = alloc_array(ht, new_max);
     for (hi = apr_hash_first(NULL, ht); hi; hi = apr_hash_next(hi)) {
@@ -190,10 +186,9 @@
         hi->this->next = new_array[i];
         new_array[i] = hi->this;
     }
+    apr_pool_cleanup_run(ht->pool, ht->array, array_cleanup);
     ht->array = new_array;
     ht->max = new_max;
-
-    apr_pool_destroy(old_array_pool);
 }
 
 APR_DECLARE_NONSTD(unsigned int) apr_hashfunc_default(const char *char_key,
@@ -306,18 +301,13 @@
 APR_DECLARE(apr_hash_t *) apr_hash_copy(apr_pool_t *pool,
                                         const apr_hash_t *orig)
 {
-    apr_pool_t *array_pool;
     apr_hash_t *ht;
     apr_hash_entry_t *new_vals;
     unsigned int i, j;
 
-    if (apr_pool_create(&array_pool, pool) != APR_SUCCESS)
-        return NULL;
-
     ht = apr_palloc(pool, sizeof(apr_hash_t) +
                     sizeof(apr_hash_entry_t) * orig->count);
     ht->pool = pool;
-    ht->array_pool = array_pool;
     ht->free = NULL;
     ht->count = orig->count;
     ht->max = orig->max;
@@ -413,7 +403,6 @@
                                                      const void *data),
                                          const void *data)
 {
-    apr_pool_t *array_pool;
     apr_hash_t *res;
     apr_hash_entry_t *new_vals = NULL;
     apr_hash_entry_t *iter;
@@ -437,12 +426,8 @@
     }
 #endif
 
-    if (apr_pool_create(&array_pool, p) != APR_SUCCESS)
-        return NULL;
-
     res = apr_palloc(p, sizeof(apr_hash_t));
     res->pool = p;
-    res->array_pool = array_pool;
     res->free = NULL;
     res->hash_func = base->hash_func;
     res->count = base->count;

Reply via email to