From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Binary search in fmgr_isbuiltin() is a bottleneck. |
Date: | 2017-10-05 14:08:39 |
Message-ID: | bd13812c-c4ae-3788-5b28-5633beed2929@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(at)anarazel(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-10-05 14:15:24 | Re: postgres_fdw bug in 9.6 |
Previous Message | Robert Haas | 2017-10-05 13:59:53 | Re: subscription worker signalling wal writer too much |