[lttng-dev] [PATCH 1/6] change default mm type

[Mathieu Desnoyers]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> When I test backend with the following commands.
> (my box is x86_64 with 4 cores/logic cpus)
> **(test with Load factor = 100% only)**
> 
> ./tests/test_urcu_hash 4 0 10 -B mmap -h $((1<<19)) -p $((1<<19))
> ./tests/test_urcu_hash 4 0 10 -B mmap -h $((1<<18)) -p $((1<<18))
> ./tests/test_urcu_hash 4 0 10 -B mmap -h $((1<<17)) -p $((1<<17))
> ./tests/test_urcu_hash 4 0 10 -B mmap -h $((1<<16)) -p $((1<<16))
>                     4readers/no writer
> 
> It shows that mmap backend is about 6% better over order backend.

Not surprising, that's a great result! :) That makes it the de-facto
default for 64-bit architectures.

> (It also shows that chunk backend is (worse than)/(the same as) order backend
> for small/large min_nr_alloc_buckets. (use -m when test)).
> 
> Note:
> "6%" and the google-perftools told us the bucket_at() is not the cirtical
> bottle neck.

Good to know too.

> 
> new strategy:
> infinite buckets size --> order mm

Yep, I guess we don't have any choice there.

> otherwise 64bits      --> mmap mm

Yes.

> otherwise    --> order or chunk mm

Hrm, I'm wondering about the tests in the code for this one, see below,

> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
>  rculfhash.c      |   14 ++++++++++++++
>  urcu/rculfhash.h |    2 +-
>  2 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/rculfhash.c b/rculfhash.c
> index 8c835de..ebf0023 100644
> --- a/rculfhash.c
> +++ b/rculfhash.c
> @@ -1280,6 +1280,9 @@ struct cds_lfht *_cds_lfht_new(unsigned long init_size,
>  	if (!init_size || (init_size & (init_size - 1)))
>  		return NULL;
>  
> +	if (!max_nr_buckets && !mm)
> +		mm = &cds_lfht_mm_order;
> +
>  	/* max_nr_buckets == 0 for order based mm means infinite */
>  	if (mm == &cds_lfht_mm_order && !max_nr_buckets)
>  		max_nr_buckets = 1UL << (MAX_TABLE_ORDER - 1);
> @@ -1293,6 +1296,17 @@ struct cds_lfht *_cds_lfht_new(unsigned long init_size,
>  	max_nr_buckets = max(max_nr_buckets, min_nr_alloc_buckets);
>  	init_size = min(init_size, max_nr_buckets);
>  
> +	if (!mm) {
> +		if (CAA_BITS_PER_LONG > 32) {
> +			mm = &cds_lfht_mm_mmap;
> +		} else if (max_nr_buckets / min_nr_alloc_buckets
> +				<= MAX_TABLE_ORDER) {

I think we should try turning the division above into a multiplication.
It will make things easier when porting to the Linux kernel for some
embedded platforms that do not have the standard udiv implemented. e.g.:

  } else if (max_nr_buckets <= MAX_TABLE_ORDER * min_nr_alloc_buckets) {

But in fact I don't understand the rationale behind using the chunk
allocator for these table. Especially given that a table with a large
min_nr_alloc_buckets (and not so much larger max_nr_buckets) will use
the chunk allocator, thus degrading its performances.

I would be tempted to fallback on the order allocator in every case, and
keep the chunk allocator as an option to explicitely specify, and of
course keep it around for page allocation when we move to the kernel.

Thanks,

Mathieu

> +			mm = &cds_lfht_mm_chunk;
> +		} else {
> +			mm = &cds_lfht_mm_order;
> +		}
> +	}
> +
>  	ht = mm->alloc_cds_lfht(min_nr_alloc_buckets, max_nr_buckets);
>  	assert(ht);
>  	assert(ht->mm == mm);
> diff --git a/urcu/rculfhash.h b/urcu/rculfhash.h
> index 6ed7c8c..de31526 100644
> --- a/urcu/rculfhash.h
> +++ b/urcu/rculfhash.h
> @@ -154,7 +154,7 @@ struct cds_lfht *cds_lfht_new(unsigned long init_size,
>  			pthread_attr_t *attr)
>  {
>  	return _cds_lfht_new(init_size, min_nr_alloc_buckets, max_nr_buckets,
> -			flags, &cds_lfht_mm_order, &rcu_flavor, attr);
> +			flags, NULL, &rcu_flavor, attr);
>  }
>  
>  /*
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com