[ltt-dev] [PATCH 6/7] rculfhash: Move key out of struct lfht_test_node

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Make struct lfht_test_node only contains basic field.
> Let user take the responsibility to handle the <key>(hash_set)
> or <key,value>(hash_map) management and calculation.

Taken care of by commits 0422d92c2d658f6093b8209f75808efd2109a110 and
b84f92d592010f01851c3fdcd5ac7d3666028146.

Thanks,

Mathieu

> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
>  rculfhash.c            |   12 +-----------
>  tests/test_urcu_hash.c |   22 +++++++++++++++-------
>  urcu/rculfhash.h       |   33 +++------------------------------
>  3 files changed, 19 insertions(+), 48 deletions(-)
> 
> diff --git a/rculfhash.c b/rculfhash.c
> index 6a8c720..a569172 100644
> --- a/rculfhash.c
> +++ b/rculfhash.c
> @@ -267,11 +267,9 @@ struct rcu_table {
>   */
>  struct cds_lfht {
>  	struct rcu_table t;
> -	cds_lfht_hash_fct hash_fct;
>  	cds_lfht_compare_fct compare_fct;
>  	unsigned long min_alloc_order;
>  	unsigned long min_alloc_size;
> -	unsigned long hash_seed;
>  	int flags;
>  	/*
>  	 * We need to put the work threads offline (QSBR) when taking this
> @@ -1336,9 +1334,7 @@ void cds_lfht_create_dummy(struct cds_lfht *ht, unsigned long size)
>  	}
>  }
>  
> -struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
> -			cds_lfht_compare_fct compare_fct,
> -			unsigned long hash_seed,
> +struct cds_lfht *_cds_lfht_new(cds_lfht_compare_fct compare_fct,
>  			unsigned long init_size,
>  			unsigned long min_alloc_size,
>  			int flags,
> @@ -1367,9 +1363,7 @@ struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
>  	ht = calloc(1, sizeof(struct cds_lfht));
>  	assert(ht);
>  	ht->flags = flags;
> -	ht->hash_fct = hash_fct;
>  	ht->compare_fct = compare_fct;
> -	ht->hash_seed = hash_seed;
>  	ht->cds_lfht_call_rcu = cds_lfht_call_rcu;
>  	ht->cds_lfht_synchronize_rcu = cds_lfht_synchronize_rcu;
>  	ht->cds_lfht_rcu_read_lock = cds_lfht_rcu_read_lock;
> @@ -1435,13 +1429,9 @@ void cds_lfht_next_duplicate(struct cds_lfht *ht, struct cds_lfht_iter *iter)
>  {
>  	struct cds_lfht_node *node, *next;
>  	unsigned long reverse_hash;
> -	void *key;
> -	size_t key_len;
>  
>  	node = iter->node;
>  	reverse_hash = node->p.reverse_hash;
> -	key = node->key;
> -	key_len = node->key_len;
>  	next = iter->next;
>  	node = clear_flag(next);
>  
> diff --git a/tests/test_urcu_hash.c b/tests/test_urcu_hash.c
> index 1cae6d6..2d2ef22 100644
> --- a/tests/test_urcu_hash.c
> +++ b/tests/test_urcu_hash.c
> @@ -109,6 +109,8 @@ struct test_data {
>  
>  struct lfht_test_node {
>  	struct cds_lfht_node node;
> +	void *key;
> +	unsigned int key_len;
>  	/* cache-cold for iteration */
>  	struct rcu_head head;
>  };
> @@ -123,7 +125,8 @@ static inline
>  void lfht_test_node_init(struct lfht_test_node *node, void *key,
>  			size_t key_len)
>  {
> -	cds_lfht_node_init(&node->node, key, key_len);
> +	node->key = key;
> +	node->key_len = key_len;
>  }
>  
>  static inline struct lfht_test_node *
> @@ -424,13 +427,18 @@ int __test_compare(void *key1, size_t key1_len,
>  static
>  int test_compare(struct cds_lfht_node *a, struct cds_lfht_node *b)
>  {
> -	return __test_compare(a->key, a->key_len, b->key, b->key_len);
> +	struct lfht_test_node *u = to_test_node(a);
> +	struct lfht_test_node *v = to_test_node(a);
> +
> +	return __test_compare(u->key, u->key_len, v->key, v->key_len);
>  }
>  
>  static
>  int test_match(struct cds_lfht_node *node, void *arg)
>  {
> -	return !__test_compare(node->key, node->key_len,
> +	struct lfht_test_node *test_node = to_test_node(node);
> +
> +	return !__test_compare(test_node->key, test_node->key_len,
>  			arg, sizeof(unsigned long));
>  }
>  
> @@ -447,7 +455,7 @@ void cds_lfht_test_lookup(struct cds_lfht *ht, void *key, size_t key_len,
>  static
>  void cds_lfht_test_add(struct cds_lfht *ht, struct lfht_test_node *node)
>  {
> -	unsigned long hash = test_hash(node->node.key, node->node.key_len, TEST_HASH_SEED);
> +	unsigned long hash = test_hash(node->key, node->key_len, TEST_HASH_SEED);
>  
>  	cds_lfht_add(ht, hash, &node->node);
>  }
> @@ -456,7 +464,7 @@ static
>  struct cds_lfht_node *cds_lfht_test_add_unique(struct cds_lfht *ht,
>  		struct lfht_test_node *node)
>  {
> -	unsigned long hash = test_hash(node->node.key, node->node.key_len, TEST_HASH_SEED);
> +	unsigned long hash = test_hash(node->key, node->key_len, TEST_HASH_SEED);
>  
>  	return cds_lfht_add_unique(ht, hash, &node->node);
>  }
> @@ -465,7 +473,7 @@ static
>  struct cds_lfht_node *cds_lfht_test_add_replace(struct cds_lfht *ht,
>  		struct lfht_test_node *node)
>  {
> -	unsigned long hash = test_hash(node->node.key, node->node.key_len, TEST_HASH_SEED);
> +	unsigned long hash = test_hash(node->key, node->key_len, TEST_HASH_SEED);
>  
>  	return cds_lfht_add_replace(ht, hash, &node->node);
>  }
> @@ -983,7 +991,7 @@ int main(int argc, char **argv)
>  	 * thread from the point of view of resize.
>  	 */
>  	rcu_register_thread();
> -	test_ht = cds_lfht_new(test_hash, test_compare, TEST_HASH_SEED,
> +	test_ht = cds_lfht_new(test_compare,
>  			init_hash_size, min_hash_alloc_size,
>  			(opt_auto_resize ? CDS_LFHT_AUTO_RESIZE : 0) |
>  			CDS_LFHT_ACCOUNTING, NULL);
> diff --git a/urcu/rculfhash.h b/urcu/rculfhash.h
> index d4c9209..cd5760e 100644
> --- a/urcu/rculfhash.h
> +++ b/urcu/rculfhash.h
> @@ -47,12 +47,6 @@ struct _cds_lfht_node {
>  } __attribute__((aligned(4)));
>  
>  /*
> - * cds_lfht_node: Contains the full key and length required to check for
> - * an actual match, and also contains an rcu_head structure that is used
> - * by RCU to track a node through a given RCU grace period.  There is an
> - * instance of _cds_lfht_node enclosed as a field within each
> - * _cds_lfht_node structure.
> - *
>   * struct cds_lfht_node can be embedded into a structure (as a field).
>   * caa_container_of() can be used to get the structure from the struct
>   * cds_lfht_node after a lookup.
> @@ -60,8 +54,6 @@ struct _cds_lfht_node {
>  struct cds_lfht_node {
>  	/* cache-hot for iteration */
>  	struct _cds_lfht_node p;          /* needs to be first field */
> -	void *key;
> -	unsigned int key_len;
>  };
>  
>  /* cds_lfht_iter: Used to track state while traversing a hash chain. */
> @@ -82,24 +74,11 @@ struct cds_lfht;
>   * Ensure reader and writer threads are registered as urcu readers.
>   */
>  
> -typedef unsigned long (*cds_lfht_hash_fct)(void *key, size_t length,
> -					unsigned long seed);
>  typedef int (*cds_lfht_compare_fct)(struct cds_lfht_node *a,
>  		struct cds_lfht_node *b);
>  typedef int (*cds_lfht_lookup_fct)(struct cds_lfht_node *node, void *arg);
>  
>  /*
> - * cds_lfht_node_init - initialize a hash table node
> - */
> -static inline
> -void cds_lfht_node_init(struct cds_lfht_node *node, void *key,
> -			size_t key_len)
> -{
> -	node->key = key;
> -	node->key_len = key_len;
> -}
> -
> -/*
>   * Hash table creation flags.
>   */
>  enum {
> @@ -110,9 +89,7 @@ enum {
>  /*
>   * _cds_lfht_new - API used by cds_lfht_new wrapper. Do not use directly.
>   */
> -struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
> -			cds_lfht_compare_fct compare_fct,
> -			unsigned long hash_seed,
> +struct cds_lfht *_cds_lfht_new(cds_lfht_compare_fct compare_fct,
>  			unsigned long init_size,
>  			unsigned long min_alloc_size,
>  			int flags,
> @@ -129,9 +106,7 @@ struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
>  
>  /*
>   * cds_lfht_new - allocate a hash table.
> - * @hash_fct: the hashing function.
>   * @compare_fct: the key comparison function.
> - * @hash_seed: the seed for hash function.
>   * @init_size: number of nodes to allocate initially. Must be power of two.
>   * @min_alloc_size: the smallest allocation size to use. Must be power of two.
>   * @flags: hash table creation flags (can be combined with bitwise or: '|').
> @@ -152,15 +127,13 @@ struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
>   * Threads calling this API need to be registered RCU read-side threads.
>   */
>  static inline
> -struct cds_lfht *cds_lfht_new(cds_lfht_hash_fct hash_fct,
> -			cds_lfht_compare_fct compare_fct,
> -			unsigned long hash_seed,
> +struct cds_lfht *cds_lfht_new(cds_lfht_compare_fct compare_fct,
>  			unsigned long init_size,
>  			unsigned long min_alloc_size,
>  			int flags,
>  			pthread_attr_t *attr)
>  {
> -	return _cds_lfht_new(hash_fct, compare_fct, hash_seed,
> +	return _cds_lfht_new(compare_fct,
>  			init_size, min_alloc_size, flags,
>  			call_rcu, synchronize_rcu, rcu_read_lock,
>  			rcu_read_unlock, rcu_thread_offline,
> -- 
> 1.7.4.4
> 

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