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

Andres Freund writes: > On 2017-10-05 17:08:39 +0300, Heikki Linnakangas wrote: >> 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: > Yea,

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

Hi, On 2017-10-05 17:08:39 +0300, Heikki Linnakangas wrote: > > 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

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 writes: Done that way. It's a bit annoying, because we've to take care to initialize the "unused" part of the array

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

On 2017-10-02 15:01:36 -0700, Andres Freund wrote: > On 2017-10-02 17:57:51 -0400, Tom Lane wrote: > > Andres Freund 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 > >

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

On 2017-10-02 17:57:51 -0400, Tom Lane wrote: > Andres Freund 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

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

Andres Freund 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

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

On 2017-09-28 19:06:27 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2017-09-28 18:52:28 -0400, Tom Lane wrote: > >> Uh, what? Access to fmgr_nbuiltins shouldn't be part of any critical path > >> anymore after this change. > > > Indeed. But the size of the the oid ->

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

Andres Freund writes: > On 2017-09-28 18:52:28 -0400, Tom Lane wrote: >> Uh, what? Access to fmgr_nbuiltins shouldn't be part of any critical path >> anymore after this change. > Indeed. But the size of the the oid -> fmgr_builtins index array is > relevant now. We could of

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

On 2017-09-28 18:52:28 -0400, Tom Lane wrote: > Andres Freund writes: > > I might be worse than you... But anyway, here's a patch doing > > so. Looking at profiles, it turned out that having the integer limits as > > extern variables in a different TU isn't a great idea. > >

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

Andres Freund writes: > I might be worse than you... But anyway, here's a patch doing > so. Looking at profiles, it turned out that having the integer limits as > extern variables in a different TU isn't a great idea. Uh, what? Access to fmgr_nbuiltins shouldn't be part of

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

On 2017-09-27 15:50:05 -0400, Tom Lane wrote: > ISTM it shouldn't be that hard to get Gen_fmgrtab.pl to emit an index > array of the sort we're talking about, along with the FmgrBuiltin array > it already prints out. I'm the world's worst Perl programmer but > I'm happy to take a stab at it if

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

I wrote: > [ we should use an index array ] Just to prove the point, I threw together the attached trivial test case, which time-trials the existing fmgr_isbuiltin implementation against both the proposed hash implementation and a simple index array. On my machine, with a repeat count of 1,

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

On 09/27/2017 05:29 PM, tushar wrote: After discussion with Jeevan Ladhe, we created a sql query which contain lots of inbuild function and tested that against pgbench    with master v/s patch and found an improvement I tested it again and found around +2% improvement ./pgbench -c 8 -j 8 -f

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

Andres Freund writes: > On 2017-09-27 15:06:15 -0400, Tom Lane wrote: >> Yeah, constructing an index table of that sort on top of the existing >> FmgrBuiltin array could be done cheaply enough at startup. It irks me >> slightly that it's not part of the read-only text

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

On 2017-09-27 15:30:45 -0400, Robert Haas wrote: > On Wed, Sep 27, 2017 at 1:40 PM, Andres Freund wrote: > > I don't think you can even measure the overhead of building the > > table. This is inserting ~8k rows in an accurately sized hashtable - a > > vanishingly small amount

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

On Wed, Sep 27, 2017 at 1:40 PM, Andres Freund wrote: > I don't think you can even measure the overhead of building the > table. This is inserting ~8k rows in an accurately sized hashtable - a > vanishingly small amount of time in comparison to the backend startup > time (and

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

Hi, On 2017-09-27 15:06:15 -0400, Tom Lane wrote: > Yeah, constructing an index table of that sort on top of the existing > FmgrBuiltin array could be done cheaply enough at startup. It irks me > slightly that it's not part of the read-only text segment, but I can't > say that there's any really

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

Andres Freund writes: > On 2017-09-27 14:58:36 -0400, Tom Lane wrote: >> Yeah, I'd been kind of wondering about that approach too. We could have, >> say, a table of int16s indexed by OIDs from 0 to , containing zero or >> an index into the table of FmgrBuiltin structs.

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

On 2017-09-27 14:58:36 -0400, Tom Lane wrote: > Andres Freund writes: > > Honestly before going there I'd rather just have > > an oid indexed array, computed at compile time. > > Yeah, I'd been kind of wondering about that approach too. We could have, > say, a table of

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

Andres Freund writes: > Honestly before going there I'd rather just have > an oid indexed array, computed at compile time. Yeah, I'd been kind of wondering about that approach too. We could have, say, a table of int16s indexed by OIDs from 0 to , containing zero or an

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

On 2017-09-27 14:40:20 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2017-09-27 13:46:50 -0400, Tom Lane wrote: > >> The other question that ought to be answered is whether a gperf hash > >> table would be faster. > > > Ugh, hacking together a quick input file for

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

Andres Freund writes: > On 2017-09-27 13:46:50 -0400, Tom Lane wrote: >> The other question that ought to be answered is whether a gperf hash >> table would be faster. > Ugh, hacking together a quick input file for gperf, I'm *far* from > convinced. The generated code does

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

Andres Freund writes: > On 2017-09-27 11:50:56 -0400, Tom Lane wrote: >> Robert Haas writes: >>> I suppose an even better approach would be to build a perfect hash >>> table at compile time so that nothing needs to be built at run-time at >>> all, but

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

On 2017-09-27 13:28:22 -0400, Robert Haas wrote: > On Wed, Sep 27, 2017 at 1:00 PM, Andres Freund wrote: > > We could relatively easily move it to be once-per-postmaster start for > > !EXEC_BACKEND builds. Constantly doing expensive binary searches is also > > pretty brute

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

On Wed, Sep 27, 2017 at 1:00 PM, Andres Freund wrote: > We could relatively easily move it to be once-per-postmaster start for > !EXEC_BACKEND builds. Constantly doing expensive binary searches is also > pretty brute force ;) I think one useful way to look at it might be -

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

On 2017-09-27 11:50:56 -0400, Tom Lane wrote: > Robert Haas writes: > > I suppose an even better approach would be to build a perfect hash > > table at compile time so that nothing needs to be built at run-time at > > all, but I'm not sure it's worth the trouble. > > Yeah,

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

Robert Haas writes: > I suppose an even better approach would be to build a perfect hash > table at compile time so that nothing needs to be built at run-time at > all, but I'm not sure it's worth the trouble. Yeah, I was wondering about that too. It would likely mean

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

On Wed, Sep 27, 2017 at 11:18 AM, Robert Haas wrote: > No, I think what Andres is saying is that we ought to build the hash > table before we ever reach this function, so that we don't have to > have a branch here to check whether it's been done. I don't see why > that's

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

On Mon, Sep 25, 2017 at 8:42 AM, Jeevan Ladhe wrote: > 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. No, I think what Andres is saying is that

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

On 09/14/2017 12:21 PM, Andres Freund wrote: Hi, Surprising myself I discovered that in workloads that do a large number of fmgr_info* lookups, fmgr_isbuiltin() is actually quite the bottleneck. After discussion with Jeevan Ladhe, we created a sql query which contain lots of inbuild function

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

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