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