From 8893215aaf714154c190c66bf7d1ce568118ec39 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 18 Jul 2012 04:53:31 +0930 Subject: talloc: use a struct for pool headers. 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 --- lib/talloc/talloc.c | 194 +++++++++++++++++++++++-------------------------- lib/talloc/testsuite.c | 2 +- 2 files changed, 90 insertions(+), 106 deletions(-) (limited to 'lib/talloc') diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c index e5fd0d28232..345f2129635 100644 --- a/lib/talloc/talloc.c +++ b/lib/talloc/talloc.c @@ -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; } } diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c index 71917038be9..eaab9d7cf0a 100644 --- a/lib/talloc/testsuite.c +++ b/lib/talloc/testsuite.c @@ -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); -- cgit