From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.desnoyers@efficios.com (Mathieu Desnoyers) Date: Fri, 4 Nov 2011 09:53:45 -0400 Subject: [ltt-dev] [PATCH 3/7] rculfhash: make cds_lfht_lookup() more generic In-Reply-To: <4EB3E57F.4020705@cn.fujitsu.com> References: <9518e81f5aee8dee44154d98ff5a183ee6a12689.1320217693.git.laijs@cn.fujitsu.com> <20111102172336.GC32359@Krystal> <4EB2A019.40006@cn.fujitsu.com> <20111104100823.GA26078@Krystal> <4EB3E57F.4020705@cn.fujitsu.com> Message-ID: <20111104135345.GA28878@Krystal> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote: > On 11/04/2011 06:08 PM, Mathieu Desnoyers wrote: > > * Lai Jiangshan (laijs at cn.fujitsu.com) wrote: > >> On 11/03/2011 01:23 AM, Mathieu Desnoyers wrote: > >>> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote: > >>>> We use hash value to determine the range of the object > >>>> and use cds_lfht_lookup_fct to lookup them. > >>>> > >>>> Signed-off-by: Lai Jiangshan > >>>> --- > >>>> rculfhash.c | 8 ++++---- > >>>> tests/test_urcu_hash.c | 25 ++++++++++++++++++++++--- > >>>> urcu/rculfhash.h | 4 +++- > >>>> 3 files changed, 29 insertions(+), 8 deletions(-) > >>>> > >>>> diff --git a/rculfhash.c b/rculfhash.c > >>>> index d1a1766..29054cd 100644 > >>>> --- a/rculfhash.c > >>>> +++ b/rculfhash.c > >>>> @@ -1391,14 +1391,14 @@ struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct, > >>>> return ht; > >>>> } > >>>> > >>>> -void cds_lfht_lookup(struct cds_lfht *ht, void *key, size_t key_len, > >>>> +void cds_lfht_lookup(struct cds_lfht *ht, unsigned long hash, > >>>> + cds_lfht_lookup_fct match, void *arg, > >>> > >>> arg -> key ? > >>> > >>> for "match", our approach is to provide this function as parameter to > >>> cds_lfht_new rather than pass it as parameter each time a lookup is > >>> performed. What is the reason why we should pass it here as a parameter? > >>> > >> > >> Allow the user define it. > >> In the test, I pass match = a function always returns true when need, > >> thus cds_lfht_lookup() returns the first node of same-hash-value-chain. > > > > This looks like an interesting debug and testing feature, but I don't > > really see why we should complicate the API to include this. Is there a > > real-life use-case you have in mind besides testing ? > > > > If we only need this for testing, we can use a static variable in the > > test program to influence the behavior of cds_lfht_test_lookup. > > > > The other patches look fine, I'm just concerned about this one because > > it adds complexity to the API which does not seem useful to end-users. > > > > I think a generic API is OK. > > Sometimes the caller don't have the key or the keys which are big > are saved separated. Example: file server > Or key comparation is too slow. > Or caller just need approximate lookup. > Or caller want to compare keys with different strategies for performance. These arguments are appealing. However, this modification seems incomplete: we would need to also modify cds_lfht_next_duplicate, and *cds_lfht_add*, so they also can specify their own compare_fct strategy, and remove the compare_fct from the lfht_new arguments, right ? Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com