[SCM] Samba Shared Repository - branch v4-0-test updated - release-4-0-0alpha2-733-g9f402c6

Michael Adam ma at sernet.de
Mon Feb 11 14:15:46 GMT 2008


Hi,

after talking to Metze, I have ported the latest changes to
talloc from s3 to s4, most notably Volkers introduction of
talloc pools.

Therefore, I think a version change is in place.
The current version is 1.1.0. So probably one should go for 1.2.0 ?

Cheers, Michael

Michael Adam wrote:
> The branch, v4-0-test has been updated
>        via  9f402c607f44663cab91cc7d2139f62b84c99cc0 (commit)
>        via  5cc7a638e93e5f3540755b441a99d4fd3c9c7134 (commit)
>        via  6deca23b6c66616fbf5ba004e9b778aa68790df6 (commit)
>        via  60ef9a84f0bd18d48e453c08aa420d17275e0881 (commit)
>        via  2a2c28584cdb65fcea8563eb3bf21fea497fdff3 (commit)
>        via  a0caedb94f6f7c62ae706e35a4c0b2876f74978d (commit)
>        via  0c829e6ee6f43299cc5889c2af3d0402256da0d0 (commit)
>       from  4cc93a98f984d322e41f403169cfa4945b469935 (commit)
> 
> http://gitweb.samba.org/?samba.git;a=shortlog;h=v4-0-test
> 
> 
> - Log -----------------------------------------------------------------
> commit 9f402c607f44663cab91cc7d2139f62b84c99cc0
> Author: Volker Lendecke <vl at samba.org>
> Date:   Thu Jan 10 11:35:40 2008 +0100
> 
>     Fix a c++ warning
> 
> commit 5cc7a638e93e5f3540755b441a99d4fd3c9c7134
> Author: Volker Lendecke <vl at samba.org>
> Date:   Thu Jan 10 11:35:17 2008 +0100
> 
>     talloc_free_children can only reset pool if it's empty
> 
> commit 6deca23b6c66616fbf5ba004e9b778aa68790df6
> Author: Volker Lendecke <vl at samba.org>
> Date:   Thu Jan 10 11:34:07 2008 +0100
> 
>     Mark talloc_pool memory for valgrind
> 
> commit 60ef9a84f0bd18d48e453c08aa420d17275e0881
> Author: Volker Lendecke <vl at samba.org>
> Date:   Sat Jan 5 18:26:54 2008 +0100
> 
>     Implement talloc_pool()
>     
>     A talloc pool is a chunk of memory that can be used as a context for further
>     talloc calls. Allocations with the pool as the parent just chew from that
>     memory by incrementing a pointer. If the talloc pool is full, then we fall back
>     to the normal system-level malloc(3) to get memory.
>     
>     The use case for talloc pools is the transient memory that is used for handling
>     a single SMB request. Incrementing a pointer will be way faster than any malloc
>     implementation.
>     
>     There is a downside of this: If you use talloc_steal() to move something out of
>     the pool, the whole pool memory is kept around until the last object inside the
>     pool is freed. So if you talloc_free() the pool, it might happen that the
>     memory is freed later. So don't hang anything off a talloc pool that should
>     live long.
>     
>     Volker
> 
> commit 2a2c28584cdb65fcea8563eb3bf21fea497fdff3
> Author: Michael Adam <obnox at samba.org>
> Date:   Wed Jan 9 01:35:43 2008 +0100
> 
>     Add a comment.
>     
>     Michael
> 
> commit a0caedb94f6f7c62ae706e35a4c0b2876f74978d
> Author: Michael Adam <obnox at samba.org>
> Date:   Wed Jan 9 01:34:21 2008 +0100
> 
>     Fix talloctort: move size check after referenced ptr check.
>     
>     Michael
> 
> commit 0c829e6ee6f43299cc5889c2af3d0402256da0d0
> Author: Volker Lendecke <vl at samba.org>
> Date:   Tue Jan 8 22:42:27 2008 +0100
> 
>     Don't shrink a talloc area if we have less than 1k to gain
> 
> -----------------------------------------------------------------------
> 
> Summary of changes:
>  source/lib/talloc/talloc.c    |  197 +++++++++++++++++++++++++++++++++++++++--
>  source/lib/talloc/talloc.h    |    1 +
>  source/lib/talloc/testsuite.c |   37 ++++++++
>  3 files changed, 228 insertions(+), 7 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/source/lib/talloc/talloc.c b/source/lib/talloc/talloc.c
> index f9aefcd..12b85f5 100644
> --- a/source/lib/talloc/talloc.c
> +++ b/source/lib/talloc/talloc.c
> @@ -60,6 +60,8 @@
>  #define TALLOC_MAGIC 0xe814ec70
>  #define TALLOC_FLAG_FREE 0x01
>  #define TALLOC_FLAG_LOOP 0x02
> +#define TALLOC_FLAG_POOL 0x04		/* This is a talloc pool */
> +#define TALLOC_FLAG_POOLMEM 0x08	/* This is allocated in a pool */
>  #define TALLOC_MAGIC_REFERENCE ((const char *)1)
>  
>  /* by default we abort when given a bad pointer (such as when talloc_free() is called 
> @@ -109,6 +111,19 @@ struct talloc_chunk {
>  	const char *name;
>  	size_t size;
>  	unsigned flags;
> +
> +	/*
> +	 * "pool" has dual use:
> +	 *
> +	 * For the talloc pool itself (i.e. TALLOC_FLAG_POOL is set), "pool"
> +	 * marks the end of the currently allocated area.
> +	 *
> +	 * For members of the pool (i.e. TALLOC_FLAG_POOLMEM is set), "pool"
> +	 * is a pointer to the struct talloc_chunk of the pool that it was
> +	 * allocated from. This way children can quickly find the pool to chew
> +	 * from.
> +	 */
> +	void *pool;
>  };
>  
>  /* 16 byte alignment seems to keep everyone happy */
> @@ -200,12 +215,87 @@ const char *talloc_parent_name(const void *ptr)
>  	return tc? tc->name : NULL;
>  }
>  
> +/*
> +  A pool carries an in-pool object count count in the first 16 bytes.
> +  bytes. This is done to support talloc_steal() to a parent outside of the
> +  pool. The count includes the pool itself, so a talloc_free() on a pool will
> +  only destroy the pool if the count has dropped to zero. A talloc_free() of a
> +  pool member will reduce the count, and eventually also call free(3) on the
> +  pool memory.
> +
> +  The object count is not put into "struct talloc_chunk" because it is only
> +  relevant for talloc pools and the alignment to 16 bytes would increase the
> +  memory footprint of each talloc chunk by those 16 bytes.
> +*/
> +
> +#define TALLOC_POOL_HDR_SIZE 16
> +
> +static unsigned int *talloc_pool_objectcount(struct talloc_chunk *tc)
> +{
> +	return (unsigned int *)((char *)tc + sizeof(struct talloc_chunk));
> +}
> +
> +/*
> +  Allocate from a pool
> +*/
> +
> +static struct talloc_chunk *talloc_alloc_pool(struct talloc_chunk *parent,
> +					      size_t size)
> +{
> +	struct talloc_chunk *pool_ctx = NULL;
> +	size_t space_left;
> +	struct talloc_chunk *result;
> +	size_t chunk_size;
> +
> +	if (parent == NULL) {
> +		return NULL;
> +	}
> +
> +	if (parent->flags & TALLOC_FLAG_POOL) {
> +		pool_ctx = parent;
> +	}
> +	else if (parent->flags & TALLOC_FLAG_POOLMEM) {
> +		pool_ctx = (struct talloc_chunk *)parent->pool;
> +	}
> +
> +	if (pool_ctx == NULL) {
> +		return NULL;
> +	}
> +
> +	space_left = ((char *)pool_ctx + TC_HDR_SIZE + pool_ctx->size)
> +		- ((char *)pool_ctx->pool);
> +
> +	/*
> +	 * Align size to 16 bytes
> +	 */
> +	chunk_size = ((size + 15) & ~15);
> +
> +	if (space_left < chunk_size) {
> +		return NULL;
> +	}
> +
> +	result = (struct talloc_chunk *)pool_ctx->pool;
> +
> +#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
> +	VALGRIND_MAKE_MEM_UNDEFINED(result, size);
> +#endif
> +
> +	pool_ctx->pool = (void *)((char *)result + chunk_size);
> +
> +	result->flags = TALLOC_MAGIC | TALLOC_FLAG_POOLMEM;
> +	result->pool = pool_ctx;
> +
> +	*talloc_pool_objectcount(pool_ctx) += 1;
> +
> +	return result;
> +}
> +
>  /* 
>     Allocate a bit of memory as a child of an existing pointer
>  */
>  static inline void *__talloc(const void *context, size_t size)
>  {
> -	struct talloc_chunk *tc;
> +	struct talloc_chunk *tc = NULL;
>  
>  	if (unlikely(context == NULL)) {
>  		context = null_context;
> @@ -215,11 +305,19 @@ static inline void *__talloc(const void *context, size_t size)
>  		return NULL;
>  	}
>  
> -	tc = (struct talloc_chunk *)malloc(TC_HDR_SIZE+size);
> -	if (unlikely(tc == NULL)) return NULL;
> +	if (context != NULL) {
> +		tc = talloc_alloc_pool(talloc_chunk_from_ptr(context),
> +				       TC_HDR_SIZE+size);
> +	}
> +
> +	if (tc == NULL) {
> +		tc = (struct talloc_chunk *)malloc(TC_HDR_SIZE+size);
> +		if (unlikely(tc == NULL)) return NULL;
> +		tc->flags = TALLOC_MAGIC;
> +		tc->pool  = NULL;
> +	}
>  
>  	tc->size = size;
> -	tc->flags = TALLOC_MAGIC;
>  	tc->destructor = NULL;
>  	tc->child = NULL;
>  	tc->name = NULL;
> @@ -246,6 +344,33 @@ static inline void *__talloc(const void *context, size_t size)
>  }
>  
>  /*
> + * Create a talloc pool
> + */
> +
> +void *talloc_pool(const void *context, size_t size)
> +{
> +	void *result = __talloc(context, size + TALLOC_POOL_HDR_SIZE);
> +	struct talloc_chunk *tc;
> +
> +	if (unlikely(result == NULL)) {
> +		return NULL;
> +	}
> +
> +	tc = talloc_chunk_from_ptr(result);
> +
> +	tc->flags |= TALLOC_FLAG_POOL;
> +	tc->pool = (char *)result + TALLOC_POOL_HDR_SIZE;
> +
> +	*talloc_pool_objectcount(tc) = 1;
> +
> +#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_NOACCESS)
> +	VALGRIND_MAKE_MEM_NOACCESS(tc->pool, size);
> +#endif
> +
> +	return result;
> +}
> +
> +/*
>    setup a destructor to be called on free of a pointer
>    the destructor should return 0 on success, or -1 on failure.
>    if the destructor fails then the free is failed, and the memory can
> @@ -420,7 +545,29 @@ static inline int _talloc_free(void *ptr)
>  	}
>  
>  	tc->flags |= TALLOC_FLAG_FREE;
> -	free(tc);
> +
> +	if (tc->flags & (TALLOC_FLAG_POOL|TALLOC_FLAG_POOLMEM)) {
> +		struct talloc_chunk *pool;
> +		unsigned int *pool_object_count;
> +
> +		pool = (tc->flags & TALLOC_FLAG_POOL)
> +			? tc : (struct talloc_chunk *)tc->pool;
> +
> +		pool_object_count = talloc_pool_objectcount(pool);
> +
> +		if (*pool_object_count == 0) {
> +			TALLOC_ABORT("Pool object count zero!");
> +		}
> +
> +		*pool_object_count -= 1;
> +
> +		if (*pool_object_count == 0) {
> +			free(pool);
> +		}
> +	}
> +	else {
> +		free(tc);
> +	}
>  	return 0;
>  }
>  
> @@ -718,6 +865,15 @@ void talloc_free_children(void *ptr)
>  			talloc_steal(new_parent, child);
>  		}
>  	}
> +
> +	if ((tc->flags & TALLOC_FLAG_POOL)
> +	    && (*talloc_pool_objectcount(tc) == 1)) {
> +		tc->pool = ((char *)tc + TC_HDR_SIZE + TALLOC_POOL_HDR_SIZE);
> +#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_NOACCESS)
> +		VALGRIND_MAKE_MEM_NOACCESS(
> +			tc->pool, tc->size - TALLOC_POOL_HDR_SIZE);
> +#endif
> +	}
>  }
>  
>  /* 
> @@ -769,6 +925,7 @@ void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *n
>  {
>  	struct talloc_chunk *tc;
>  	void *new_ptr;
> +	bool malloced = false;
>  
>  	/* size zero is equivalent to free() */
>  	if (unlikely(size == 0)) {
> @@ -792,6 +949,12 @@ void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *n
>  		return NULL;
>  	}
>  
> +	/* don't shrink if we have less than 1k to gain */
> +	if ((size < tc->size) && ((tc->size - size) < 1024)) {
> +		tc->size = size;
> +		return ptr;
> +	}
> +
>  	/* by resetting magic we catch users of the old memory */
>  	tc->flags |= TALLOC_FLAG_FREE;
>  
> @@ -802,7 +965,24 @@ void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *n
>  		free(tc);
>  	}
>  #else
> -	new_ptr = realloc(tc, size + TC_HDR_SIZE);
> +	if (tc->flags & TALLOC_FLAG_POOLMEM) {
> +
> +		new_ptr = talloc_alloc_pool(tc, size + TC_HDR_SIZE);
> +		*talloc_pool_objectcount((struct talloc_chunk *)
> +					 (tc->pool)) -= 1;
> +
> +		if (new_ptr == NULL) {
> +			new_ptr = malloc(TC_HDR_SIZE+size);
> +			malloced = true;
> +		}
> +
> +		if (new_ptr) {
> +			memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE);
> +		}
> +	}
> +	else {
> +		new_ptr = realloc(tc, size + TC_HDR_SIZE);
> +	}
>  #endif
>  	if (unlikely(!new_ptr)) {	
>  		tc->flags &= ~TALLOC_FLAG_FREE; 
> @@ -810,7 +990,10 @@ void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *n
>  	}
>  
>  	tc = (struct talloc_chunk *)new_ptr;
> -	tc->flags &= ~TALLOC_FLAG_FREE; 
> +	tc->flags &= ~TALLOC_FLAG_FREE;
> +	if (malloced) {
> +		tc->flags &= ~TALLOC_FLAG_POOLMEM;
> +	}
>  	if (tc->parent) {
>  		tc->parent->child = tc;
>  	}
> diff --git a/source/lib/talloc/talloc.h b/source/lib/talloc/talloc.h
> index e103391..5431971 100644
> --- a/source/lib/talloc/talloc.h
> +++ b/source/lib/talloc/talloc.h
> @@ -116,6 +116,7 @@ typedef void TALLOC_CTX;
>  
>  /* The following definitions come from talloc.c  */
>  void *_talloc(const void *context, size_t size);
> +void *talloc_pool(const void *context, size_t size);
>  void _talloc_set_destructor(const void *ptr, int (*destructor)(void *));
>  int talloc_increase_ref_count(const void *ptr);
>  size_t talloc_reference_count(const void *ptr);
> diff --git a/source/lib/talloc/testsuite.c b/source/lib/talloc/testsuite.c
> index e16c91f..fedbda9 100644
> --- a/source/lib/talloc/testsuite.c
> +++ b/source/lib/talloc/testsuite.c
> @@ -813,6 +813,25 @@ static bool test_speed(void)
>  
>  	talloc_free(ctx);
>  
> +	ctx = talloc_pool(NULL, 1024);
> +
> +	tv = timeval_current();
> +	count = 0;
> +	do {
> +		void *p1, *p2, *p3;
> +		for (i=0;i<loop;i++) {
> +			p1 = talloc_size(ctx, loop % 100);
> +			p2 = talloc_strdup(p1, "foo bar");
> +			p3 = talloc_size(p1, 300);
> +			talloc_free_children(ctx);
> +		}
> +		count += 3 * loop;
> +	} while (timeval_elapsed(&tv) < 5.0);
> +
> +	talloc_free(ctx);
> +
> +	fprintf(stderr, "talloc_pool: %.0f ops/sec\n", count/timeval_elapsed(&tv));
> +
>  	tv = timeval_current();
>  	count = 0;
>  	do {
> @@ -1066,6 +1085,23 @@ static bool test_autofree(void)
>  	return true;
>  }
>  
> +static bool test_pool(void)
> +{
> +	void *pool;
> +	void *p1, *p2, *p3, *p4;
> +
> +	pool = talloc_pool(NULL, 1024);
> +
> +	p1 = talloc_size(pool, 80);
> +	p2 = talloc_size(pool, 20);
> +	p3 = talloc_size(p1, 50);
> +	p4 = talloc_size(p3, 1000);
> +
> +	talloc_free(pool);
> +
> +	return true;
> +}
> +
>  struct torture_context;
>  bool torture_local_talloc(struct torture_context *tctx)
>  {
> @@ -1094,6 +1130,7 @@ bool torture_local_talloc(struct torture_context *tctx)
>  	ret &= test_free_parent_deny_child(); 
>  	ret &= test_talloc_ptrtype();
>  	ret &= test_talloc_free_in_destructor();
> +	ret &= test_pool();
>  
>  	if (ret) {
>  		ret &= test_speed();
> 
> 
> -- 
> Samba Shared Repository

-- 

i.A. Michael Adam

-- 
Michael Adam <ma at sernet.de>
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.SerNet.DE, mailto: Info @ SerNet.DE
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20080211/a2094900/attachment.bin


More information about the samba-technical mailing list