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

[Lai Jiangshan]

On 12/02/2011 09:04 PM, Mathieu Desnoyers wrote:
> * 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.
> 

I'm OK. Could you change the patch as you suggested and merge it.

Thanks,
Lai