talloc: use a struct for pool headers.
authorRusty Russell <rusty@rustcorp.com.au>
Tue, 17 Jul 2012 19:23:31 +0000 (04:53 +0930)
committerRusty Russell <rusty@rustcorp.com.au>
Tue, 17 Jul 2012 19:23:31 +0000 (04:53 +0930)
This neatens the code a bit (we should do a similar thing for all the
TALLOC_CHUNK macros).

Two subtler changes:
(1) As a result of the struct, we actually pack object_count into the
    talloc header on 32-bit platforms (since the header is 40 bytes, but
    needs to be 16-byte aligned).
(2) I avoid VALGRIND_MAKE_MEM_UNDEFINED on memmove when we resize the
    only entry in a pool; that's done later anyway.

With -O2 on my 11.04 Ubuntu 32-bit x86 laptop, the talloc_pool speed as
measured by testsuite.c actually increases 10%.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
lib/talloc/talloc.c
lib/talloc/testsuite.c

index e5fd0d28232a654749b020e5e5c4bec100f31a0b..345f212963510be1289e15f9055b569e0c23c2a0 100644 (file)
@@ -437,49 +437,50 @@ _PUBLIC_ const char *talloc_parent_name(const void *ptr)
   memory footprint of each talloc chunk by those 16 bytes.
 */
 
-#define TALLOC_POOL_HDR_SIZE 16
+union talloc_pool_chunk {
+       /* This lets object_count nestle into 16-byte padding of talloc_chunk,
+        * on 32-bit platforms. */
+       struct tc_pool_hdr {
+               struct talloc_chunk c;
+               unsigned int object_count;
+       } hdr;
+       /* This makes it always 16 byte aligned. */
+       char pad[TC_ALIGN16(sizeof(struct tc_pool_hdr))];
+};
 
-#define TC_POOL_SPACE_LEFT(_pool_tc) \
-       PTR_DIFF(TC_HDR_SIZE + (_pool_tc)->size + (char *)(_pool_tc), \
-                (_pool_tc)->pool)
+static void *tc_pool_end(union talloc_pool_chunk *pool_tc)
+{
+       return (char *)pool_tc + TC_HDR_SIZE + pool_tc->hdr.c.size;
+}
 
-#define TC_POOL_FIRST_CHUNK(_pool_tc) \
-       ((void *)(TC_HDR_SIZE + TALLOC_POOL_HDR_SIZE + (char *)(_pool_tc)))
+static size_t tc_pool_space_left(union talloc_pool_chunk *pool_tc)
+{
+       return (char *)tc_pool_end(pool_tc) - (char *)pool_tc->hdr.c.pool;
+}
 
-#define TC_POOLMEM_CHUNK_SIZE(_tc) \
-       TC_ALIGN16(TC_HDR_SIZE + (_tc)->size)
+static void *tc_pool_first_chunk(union talloc_pool_chunk *pool_tc)
+{
+       return pool_tc + 1;
+}
 
-#define TC_POOLMEM_NEXT_CHUNK(_tc) \
-       ((void *)(TC_POOLMEM_CHUNK_SIZE(tc) + (char*)(_tc)))
+/* If tc is inside a pool, this gives the next neighbour. */
+static void *tc_next_chunk(struct talloc_chunk *tc)
+{
+       return (char *)tc + TC_ALIGN16(TC_HDR_SIZE + tc->size);
+}
 
 /* Mark the whole remaining pool as not accessable */
-#define TC_INVALIDATE_FILL_POOL(_pool_tc) do { \
-       if (unlikely(talloc_fill.enabled)) { \
-               size_t _flen = TC_POOL_SPACE_LEFT(_pool_tc); \
-               char *_fptr = (char *)(_pool_tc)->pool; \
-               memset(_fptr, talloc_fill.fill_value, _flen); \
-       } \
-} while(0)
+static void tc_invalidate_pool(union talloc_pool_chunk *pool_tc)
+{
+       size_t flen = tc_pool_space_left(pool_tc);
+
+       if (unlikely(talloc_fill.enabled)) {
+               memset(pool_tc->hdr.c.pool, talloc_fill.fill_value, flen);
+       }
 
 #if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_NOACCESS)
-/* Mark the whole remaining pool as not accessable */
-#define TC_INVALIDATE_VALGRIND_POOL(_pool_tc) do { \
-       size_t _flen = TC_POOL_SPACE_LEFT(_pool_tc); \
-       char *_fptr = (char *)(_pool_tc)->pool; \
-       VALGRIND_MAKE_MEM_NOACCESS(_fptr, _flen); \
-} while(0)
-#else
-#define TC_INVALIDATE_VALGRIND_POOL(_pool_tc) do { } while (0)
+       VALGRIND_MAKE_MEM_NOACCESS(pool_tc->hdr.c.pool, flen);
 #endif
-
-#define TC_INVALIDATE_POOL(_pool_tc) do { \
-       TC_INVALIDATE_FILL_POOL(_pool_tc); \
-       TC_INVALIDATE_VALGRIND_POOL(_pool_tc); \
-} while (0)
-
-static unsigned int *talloc_pool_objectcount(struct talloc_chunk *tc)
-{
-       return (unsigned int *)((char *)tc + TC_HDR_SIZE);
 }
 
 /*
@@ -489,7 +490,7 @@ static unsigned int *talloc_pool_objectcount(struct talloc_chunk *tc)
 static struct talloc_chunk *talloc_alloc_pool(struct talloc_chunk *parent,
                                              size_t size)
 {
-       struct talloc_chunk *pool_ctx = NULL;
+       union talloc_pool_chunk *pool_ctx = NULL;
        size_t space_left;
        struct talloc_chunk *result;
        size_t chunk_size;
@@ -499,17 +500,17 @@ static struct talloc_chunk *talloc_alloc_pool(struct talloc_chunk *parent,
        }
 
        if (parent->flags & TALLOC_FLAG_POOL) {
-               pool_ctx = parent;
+               pool_ctx = (union talloc_pool_chunk *)parent;
        }
        else if (parent->flags & TALLOC_FLAG_POOLMEM) {
-               pool_ctx = (struct talloc_chunk *)parent->pool;
+               pool_ctx = (union talloc_pool_chunk *)parent->pool;
        }
 
        if (pool_ctx == NULL) {
                return NULL;
        }
 
-       space_left = TC_POOL_SPACE_LEFT(pool_ctx);
+       space_left = tc_pool_space_left(pool_ctx);
 
        /*
         * Align size to 16 bytes
@@ -520,18 +521,18 @@ static struct talloc_chunk *talloc_alloc_pool(struct talloc_chunk *parent,
                return NULL;
        }
 
-       result = (struct talloc_chunk *)pool_ctx->pool;
+       result = (struct talloc_chunk *)pool_ctx->hdr.c.pool;
 
 #if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
        VALGRIND_MAKE_MEM_UNDEFINED(result, size);
 #endif
 
-       pool_ctx->pool = (void *)((char *)result + chunk_size);
+       pool_ctx->hdr.c.pool = (void *)((char *)result + chunk_size);
 
        result->flags = TALLOC_MAGIC | TALLOC_FLAG_POOLMEM;
        result->pool = pool_ctx;
 
-       *talloc_pool_objectcount(pool_ctx) += 1;
+       pool_ctx->hdr.object_count++;
 
        return result;
 }
@@ -595,21 +596,20 @@ static inline void *__talloc(const void *context, size_t size)
 
 _PUBLIC_ void *talloc_pool(const void *context, size_t size)
 {
-       void *result = __talloc(context, size + TALLOC_POOL_HDR_SIZE);
-       struct talloc_chunk *tc;
+       union talloc_pool_chunk *pool_tc;
+       void *result = __talloc(context, sizeof(*pool_tc) - TC_HDR_SIZE + size);
 
        if (unlikely(result == NULL)) {
                return NULL;
        }
 
-       tc = talloc_chunk_from_ptr(result);
-
-       tc->flags |= TALLOC_FLAG_POOL;
-       tc->pool = TC_POOL_FIRST_CHUNK(tc);
+       pool_tc = (union talloc_pool_chunk *)talloc_chunk_from_ptr(result);
+       pool_tc->hdr.c.flags |= TALLOC_FLAG_POOL;
+       pool_tc->hdr.c.pool = tc_pool_first_chunk(pool_tc);
 
-       *talloc_pool_objectcount(tc) = 1;
+       pool_tc->hdr.object_count = 1;
 
-       TC_INVALIDATE_POOL(tc);
+       tc_invalidate_pool(pool_tc);
 
        return result;
 }
@@ -712,12 +712,11 @@ static void *_talloc_steal_internal(const void *new_ctx, const void *ptr);
 static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
                                        const char *location)
 {
-       struct talloc_chunk *pool;
+       union talloc_pool_chunk *pool;
        void *next_tc;
-       unsigned int *pool_object_count;
 
-       pool = (struct talloc_chunk *)tc->pool;
-       next_tc = TC_POOLMEM_NEXT_CHUNK(tc);
+       pool = (union talloc_pool_chunk *)tc->pool;
+       next_tc = tc_next_chunk(tc);
 
        tc->flags |= TALLOC_FLAG_FREE;
 
@@ -729,16 +728,15 @@ static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
 
        TC_INVALIDATE_FULL_CHUNK(tc);
 
-       pool_object_count = talloc_pool_objectcount(pool);
-
-       if (unlikely(*pool_object_count == 0)) {
+       if (unlikely(pool->hdr.object_count == 0)) {
                talloc_abort("Pool object count zero!");
                return;
        }
 
-       *pool_object_count -= 1;
+       pool->hdr.object_count--;
 
-       if (unlikely(*pool_object_count == 1 && !(pool->flags & TALLOC_FLAG_FREE))) {
+       if (unlikely(pool->hdr.object_count == 1
+                    && !(pool->hdr.c.flags & TALLOC_FLAG_FREE))) {
                /*
                 * if there is just one object left in the pool
                 * and pool->flags does not have TALLOC_FLAG_FREE,
@@ -746,25 +744,25 @@ static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
                 * the rest is available for new objects
                 * again.
                 */
-               pool->pool = TC_POOL_FIRST_CHUNK(pool);
-               TC_INVALIDATE_POOL(pool);
-       } else if (unlikely(*pool_object_count == 0)) {
+               pool->hdr.c.pool = tc_pool_first_chunk(pool);
+               tc_invalidate_pool(pool);
+       } else if (unlikely(pool->hdr.object_count == 0)) {
                /*
                 * we mark the freed memory with where we called the free
                 * from. This means on a double free error we can report where
                 * the first free came from
                 */
-               pool->name = location;
+               pool->hdr.c.name = location;
 
-               TC_INVALIDATE_FULL_CHUNK(pool);
+               TC_INVALIDATE_FULL_CHUNK(&pool->hdr.c);
                free(pool);
-       } else if (pool->pool == next_tc) {
+       } else if (pool->hdr.c.pool == next_tc) {
                /*
                 * if pool->pool still points to end of
                 * 'tc' (which is stored in the 'next_tc' variable),
                 * we can reclaim the memory of 'tc'.
                 */
-               pool->pool = tc;
+               pool->hdr.c.pool = tc;
        }
 }
 
@@ -854,18 +852,15 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
        tc->name = location;
 
        if (tc->flags & TALLOC_FLAG_POOL) {
-               unsigned int *pool_object_count;
-
-               pool_object_count = talloc_pool_objectcount(tc);
+               union talloc_pool_chunk *pool = (union talloc_pool_chunk *)tc;
 
-               if (unlikely(*pool_object_count == 0)) {
+               if (unlikely(pool->hdr.object_count == 0)) {
                        talloc_abort("Pool object count zero!");
                        return 0;
                }
 
-               *pool_object_count -= 1;
-
-               if (unlikely(*pool_object_count == 0)) {
+               pool->hdr.object_count--;
+               if (unlikely(pool->hdr.object_count == 0)) {
                        TC_INVALIDATE_FULL_CHUNK(tc);
                        free(tc);
                }
@@ -1380,7 +1375,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
        struct talloc_chunk *tc;
        void *new_ptr;
        bool malloced = false;
-       struct talloc_chunk *pool_tc = NULL;
+       union talloc_pool_chunk *pool_tc = NULL;
 
        /* size zero is equivalent to free() */
        if (unlikely(size == 0)) {
@@ -1409,20 +1404,21 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
                return NULL;
        }
 
-       /* don't let anybody try to realloc a talloc_pool */
+       /* handle realloc inside a talloc_pool */
        if (unlikely(tc->flags & TALLOC_FLAG_POOLMEM)) {
-               pool_tc = (struct talloc_chunk *)tc->pool;
+               pool_tc = (union talloc_pool_chunk *)tc->pool;
        }
 
 #if (ALWAYS_REALLOC == 0)
        /* don't shrink if we have less than 1k to gain */
        if (size < tc->size) {
                if (pool_tc) {
-                       void *next_tc = TC_POOLMEM_NEXT_CHUNK(tc);
+                       void *next_tc = tc_next_chunk(tc);
                        TC_INVALIDATE_SHRINK_CHUNK(tc, size);
                        tc->size = size;
-                       if (next_tc == pool_tc->pool) {
-                               pool_tc->pool = TC_POOLMEM_NEXT_CHUNK(tc);
+                       if (next_tc == pool_tc->hdr.c.pool) {
+                               /* note: tc->size has changed, so this works */
+                               pool_tc->hdr.c.pool = tc_next_chunk(tc);
                        }
                        return ptr;
                } else if ((tc->size - size) < 1024) {
@@ -1455,7 +1451,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
 #if ALWAYS_REALLOC
        if (pool_tc) {
                new_ptr = talloc_alloc_pool(tc, size + TC_HDR_SIZE);
-               *talloc_pool_objectcount(pool_tc) -= 1;
+               pool_tc->hdr.object_count--;
 
                if (new_ptr == NULL) {
                        new_ptr = malloc(TC_HDR_SIZE+size);
@@ -1475,14 +1471,14 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
        }
 #else
        if (pool_tc) {
-               void *next_tc = TC_POOLMEM_NEXT_CHUNK(tc);
-               size_t old_chunk_size = TC_POOLMEM_CHUNK_SIZE(tc);
+               void *next_tc = tc_next_chunk(tc);
+               size_t old_chunk_size = TC_ALIGN16(TC_HDR_SIZE + tc->size);
                size_t new_chunk_size = TC_ALIGN16(TC_HDR_SIZE + size);
                size_t space_needed;
                size_t space_left;
-               unsigned int chunk_count = *talloc_pool_objectcount(pool_tc);
+               unsigned int chunk_count = pool_tc->hdr.object_count;
 
-               if (!(pool_tc->flags & TALLOC_FLAG_FREE)) {
+               if (!(pool_tc->hdr.c.flags & TALLOC_FLAG_FREE)) {
                        chunk_count -= 1;
                }
 
@@ -1491,27 +1487,15 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
                         * optimize for the case where 'tc' is the only
                         * chunk in the pool.
                         */
+                       char *start = tc_pool_first_chunk(pool_tc);
                        space_needed = new_chunk_size;
-                       space_left = pool_tc->size - TALLOC_POOL_HDR_SIZE;
+                       space_left = (char *)tc_pool_end(pool_tc) - start;
 
                        if (space_left >= space_needed) {
                                size_t old_used = TC_HDR_SIZE + tc->size;
                                size_t new_used = TC_HDR_SIZE + size;
-                               pool_tc->pool = TC_POOL_FIRST_CHUNK(pool_tc);
-#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
-                               /*
-                                * we need to prepare the memmove into
-                                * the unaccessable area.
-                                */
-                               {
-                                       size_t diff = PTR_DIFF(tc, pool_tc->pool);
-                                       size_t flen = MIN(diff, old_used);
-                                       char *fptr = (char *)pool_tc->pool;
-                                       VALGRIND_MAKE_MEM_UNDEFINED(fptr, flen);
-                               }
-#endif
-                               memmove(pool_tc->pool, tc, old_used);
-                               new_ptr = pool_tc->pool;
+                               new_ptr = start;
+                               memmove(new_ptr, tc, old_used);
 
                                tc = (struct talloc_chunk *)new_ptr;
                                TC_UNDEFINE_GROW_CHUNK(tc, size);
@@ -1521,11 +1505,11 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
                                 * because we want to invalidate the padding
                                 * too.
                                 */
-                               pool_tc->pool = new_used + (char *)new_ptr;
-                               TC_INVALIDATE_POOL(pool_tc);
+                               pool_tc->hdr.c.pool = new_used + (char *)new_ptr;
+                               tc_invalidate_pool(pool_tc);
 
                                /* now the aligned pointer */
-                               pool_tc->pool = new_chunk_size + (char *)new_ptr;
+                               pool_tc->hdr.c.pool = new_chunk_size + (char *)new_ptr;
                                goto got_new_ptr;
                        }
 
@@ -1539,19 +1523,19 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
                        return ptr;
                }
 
-               if (next_tc == pool_tc->pool) {
+               if (next_tc == pool_tc->hdr.c.pool) {
                        /*
                         * optimize for the case where 'tc' is the last
                         * chunk in the pool.
                         */
                        space_needed = new_chunk_size - old_chunk_size;
-                       space_left = TC_POOL_SPACE_LEFT(pool_tc);
+                       space_left = tc_pool_space_left(pool_tc);
 
                        if (space_left >= space_needed) {
                                TC_UNDEFINE_GROW_CHUNK(tc, size);
                                tc->flags &= ~TALLOC_FLAG_FREE;
                                tc->size = size;
-                               pool_tc->pool = TC_POOLMEM_NEXT_CHUNK(tc);
+                               pool_tc->hdr.c.pool = tc_next_chunk(tc);
                                return ptr;
                        }
                }
index 71917038be9f01ee466987052b44ac63fca1f192..eaab9d7cf0a8ad2ab72acc12d5add0eba1f0ba4a 100644 (file)
@@ -848,7 +848,7 @@ static bool test_speed(void)
                        p1 = talloc_size(ctx, loop % 100);
                        p2 = talloc_strdup(p1, "foo bar");
                        p3 = talloc_size(p1, 300);
-                       talloc_free_children(ctx);
+                       talloc_free(p1);
                }
                count += 3 * loop;
        } while (timeval_elapsed(&tv) < 5.0);