Re: type cache cleanup improvements

Поиск
Список
Период
Сортировка
От Teodor Sigaev
Тема Re: type cache cleanup improvements
Дата
Msg-id 4ed6a0d7-4297-4fda-8df9-aa28b2526e4a@sigaev.ru
обсуждение исходный текст
Ответ на Re: type cache cleanup improvements  (Michael Paquier <michael@paquier.xyz>)
Ответы Re: type cache cleanup improvements  (Michael Paquier <michael@paquier.xyz>)
Список pgsql-hackers
> 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/
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Heikki Linnakangas
Дата:
Сообщение: Re: BitmapHeapScan streaming read user and prelim refactoring
Следующее
От: Bharath Rupireddy
Дата:
Сообщение: Re: Add new error_action COPY ON_ERROR "log"