Re: [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.

Поиск
Список
Период
Сортировка
От Jeevan Ladhe
Тема Re: [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.
Дата
Msg-id CAOgcT0OfsvS551au3XfMnbV9ZLqq=ybB_BaX63d2VtQgBYWZow@mail.gmail.com
обсуждение исходный текст
Ответ на [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.  (Andres Freund <andres@anarazel.de>)
Ответы Re: [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Дерево обсуждения
Hi Andres,

Another idea would be to have an array of FmgrBuiltin*, that we index by
oid. That'd not be super small though, given that the space for function
oids is sparse.


I totally agree here, as the oids are very much scattered having an
array is not feasible here.
 
Thus what I've instead done is replacing the binary search in
fmgr_isbuiltin() with a simplehash.h style hashtable. After that the
lookup is still visible in the profile, but far less prominent.

I'd like to move the hash creation out of fmgr_isbuiltin (to avoid
having to check whether it's already created), but there's currently no
convenient place to create the hash from.   Now that we don't rely on
the sortedness of fmgrtab.c we could remove a few lines from
Gen_fmgrtab.pl, but I don't quite see the advantage. If we were
interested in a faster by-name lookup we could sort it by name, but
that'd be better solved by another hashtable...

I looked into these patches.
Seems patch 004 is already committed, commit id: 791961f59b792fbd4f0a992d3ccab47298e79103

About patch 0005:
The patch still applies cleanly.
There are no failures in ‘make check’

+ /* TODO: it'd be better if this were done separately */
+ if (unlikely(oid2builtins == NULL))
  {
- int i = (high + low) / 2;
- const FmgrBuiltin *ptr = &fmgr_builtins[i];
+ int i;
 
- if (id == ptr->foid)
- return ptr;
- else if (id > ptr->foid)
- low = i + 1;
- else
- high = i - 1;
+ oid2builtins = oid2builtins_create(TopMemoryContext,
+   fmgr_nbuiltins,
+   NULL);
+ for (i = 0; i < fmgr_nbuiltins; i++)
+ {
+ const FmgrBuiltin *ptr = &fmgr_builtins[i];
+ bool found;
+
+ entry = oid2builtins_insert(oid2builtins, ptr->foid, &found);
+ Assert(!found);
+ entry->builtin = ptr;
+ }

As Andres has already pointed, may be we want to move above code in a separate
function, and just call that function here in case the hash is not already built.

Further I am thinking about doing some performance testing, Andres can you please
point me how did you test it and what perf numbers you saw for this particular patch(0005).

Regards,
Jeevan Ladhe 

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

Предыдущее
От: Kyotaro HORIGUCHI
Дата:
Сообщение: Re: [HACKERS] GUC for cleanup indexes threshold.
Следующее
От: Shubham Barai
Дата:
Сообщение: Re: [HACKERS] GSoC 2017: weekly progress reports (week 4) and patchfor hash index