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

On 10/04/2017 10:33 AM, Andres Freund wrote:
> On 2017-10-02 15:01:36 -0700, Andres Freund wrote:
>> On 2017-10-02 17:57:51 -0400, Tom Lane wrote:
>>> Andres Freund <andres@anarazel.de> writes:
>>>> Done that way. It's a bit annoying, because we've to take care to
>>>> initialize the "unused" part of the array with a valid signalling it's
>>>> an unused mapping. Can't use 0 for that because fmgr_builtins[0] is a
>>>> valid entry.
>>>
>>> The prototype code I posted further upthread just used -1 as the "unused"
>>> marker. There's no reason the array can't be int16 rather than uint16,
>>> and "if (index < 0)" is probably a faster test anyway.
>>
>> Right, but whether we use -1 or UINT16_MAX or such doesn't matter. The
>> relevant bit is that we can't use 0, so we can't rely on the rest of the
>> array being zero initialized, but instead of to initialize all of it
>> explicitly.  I've no real feelings about using -1 or UINT16_MAX - I'd be
>> very surprised if there's any sort of meaningful performance difference.
> 
> I pushed a further cleaned up version of these two patches.  If you see
> a way to avoid initializing the "trailing" part of the
> fmgr_builtin_oid_index in a different manner, I'm all ears ;)

You could put a dummy entry at fmgr_builtins[0].

BTW, there's some alignment padding in FmgrBuiltin, when 
MAXIMUM_ALIGNOF==8. You could easily shrink the struct from 32 to 24 
bytes by moving funcName to the end of the struct:

--- a/src/include/utils/fmgrtab.h
+++ b/src/include/utils/fmgrtab.h
@@ -25,11 +25,11 @@ typedef struct {     Oid            foid;            /* OID of the function */
-    const char *funcName;        /* C name of the function */     short        nargs;            /* 0..FUNC_MAX_ARGS,
or-1 if variable count */     bool        strict;            /* T if function is "strict" */     bool        retset;
       /* T if function returns a set */     PGFunction    func;            /* pointer to compiled function */
 
+    const char *funcName;        /* C name of the function */ } FmgrBuiltin;
 extern const FmgrBuiltin fmgr_builtins[];

If we care about cache efficiency here, we could move funcName out of 
the fmgr_builtins array, to a separate array of the same size. I believe 
funcName is only used when you create an internal-language function with 
CREATE FUNCTION, and having it in a separate array shouldn't hurt those 
lookups.

- Heikki


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers