Postgres Pro
Downloads
Home   >   mailing lists

Re: [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck. - Mailing list pgsql-hackers

From Jeevan Ladhe
Subject Re: [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.
Date
Msg-id CAOgcT0OfsvS551au3XfMnbV9ZLqq=ybB_BaX63d2VtQgBYWZow@mail.gmail.com
Whole thread Raw
In response to [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.  (Andres Freund <andres@anarazel.de>)
Responses Re: [HACKERS] Binary search in fmgr_isbuiltin() is a bottleneck.
List pgsql-hackers
Tree view
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 by date:

Previous
From: Kyotaro HORIGUCHI
Date:
Subject: Re: [HACKERS] GUC for cleanup indexes threshold.
Next
From: Shubham Barai
Date:
Subject: Re: [HACKERS] GSoC 2017: weekly progress reports (week 4) and patchfor hash index