> I think that this patch should be split for clarity, as there are a
> few things that are independently useful. I guess something like
> that:
Done, all patches should be applied consequentially.
> - The typcache changes.
01-map_rel_to_type.v5.patch adds map relation to its type
> - Introduction of hash_initial_lookup(), that simplifies 3 places of
> dynahash.c where the same code is used. The routine should be
> inlined.
> - The split in hash_seq_search to force a different type of search is
> weird, complicating the dynahash interface by hiding what seems like a
> search mode. Rather than hasHashvalue that's hidden in the middle of
> HASH_SEQ_STATUS, could it be better to have an entirely different API
> for the search? That should be a patch on its own, as well.
02-hash_seq_init_with_hash_value.v5.patch - introduces a
hash_seq_init_with_hash_value() method. hash_initial_lookup() is marked as
inline, but I suppose, modern compilers are smart enough to inline it automatically.
Using separate interface for scanning hash with hash value will make scan code
more ugly in case when we need to use special value of hash value as it is done
in cache's scans. Look, instead of this simplified code:
if (hashvalue == 0)
hash_seq_init(&status, TypeCacheHash);
else
hash_seq_init_with_hash_value(&status, TypeCacheHash, hashvalue);
while ((typentry = hash_seq_search(&status)) != NULL) {
...
}
we will need to code something like that:
if (hashvalue == 0)
{
hash_seq_init(&status, TypeCacheHash);
while ((typentry = hash_seq_search(&status)) != NULL) {
...
}
}
else
{
hash_seq_init_with_hash_value(&status, TypeCacheHash, hashvalue);
while ((typentry = hash_seq_search_with_hash_value(&status)) != NULL) {
...
}
}
Or I didn't understand you.
I thought about integrate check inside existing loop in hash_seq_search() :
+ rerun:
if ((curElem = status->curEntry) != NULL)
{
/* Continuing scan of curBucket... */
status->curEntry = curElem->link;
if (status->curEntry == NULL) /* end of this bucket */
+ {
+ if (status->hasHashvalue)
+ hash_seq_term(status);
++status->curBucket;
+ }
+ else if (status->hasHashvalue && status->hashvalue !=
+ curElem->hashvalue)
+ goto rerun;
return (void *) ELEMENTKEY(curElem);
}
But for me it looks weird and adds some checks which will takes some CPU time.
03-att_with_hash_value.v5.patch - adds usage of previous patch.
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
