Commit 212e6f34 authored by Andres Freund's avatar Andres Freund

Replace binary search in fmgr_isbuiltin with a lookup array.

Turns out we have enough functions that the binary search is quite
noticeable in profiles.

Thus have Gen_fmgrtab.pl build a new mapping from a builtin function's
oid to an index in the existing fmgr_builtins array. That keeps the
additional memory usage at a reasonable amount.

Author: Andres Freund, with input from Tom Lane
Discussion: https://postgr.es/m/20170914065128.a5sk7z4xde5uy3ei@alap3.anarazel.de
parent 18f791ab
...@@ -21,6 +21,8 @@ use warnings; ...@@ -21,6 +21,8 @@ use warnings;
# Collect arguments # Collect arguments
my $infile; # pg_proc.h my $infile; # pg_proc.h
my $output_path = ''; my $output_path = '';
my @include_path;
while (@ARGV) while (@ARGV)
{ {
my $arg = shift @ARGV; my $arg = shift @ARGV;
...@@ -32,6 +34,10 @@ while (@ARGV) ...@@ -32,6 +34,10 @@ while (@ARGV)
{ {
$output_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV; $output_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
} }
elsif ($arg =~ /^-I/)
{
push @include_path, length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
}
else else
{ {
usage(); usage();
...@@ -44,6 +50,13 @@ if ($output_path ne '' && substr($output_path, -1) ne '/') ...@@ -44,6 +50,13 @@ if ($output_path ne '' && substr($output_path, -1) ne '/')
$output_path .= '/'; $output_path .= '/';
} }
# Sanity check arguments.
die "No input files.\n" if !$infile;
die "No include path; you must specify -I at least once.\n" if !@include_path;
my $FirstBootstrapObjectId =
Catalog::FindDefinedSymbol('access/transam.h', \@include_path, 'FirstBootstrapObjectId');
# Read all the data from the include/catalog files. # Read all the data from the include/catalog files.
my $catalogs = Catalog::Catalogs($infile); my $catalogs = Catalog::Catalogs($infile);
...@@ -176,6 +189,7 @@ qq|/*------------------------------------------------------------------------- ...@@ -176,6 +189,7 @@ qq|/*-------------------------------------------------------------------------
#include "postgres.h" #include "postgres.h"
#include "access/transam.h"
#include "utils/fmgrtab.h" #include "utils/fmgrtab.h"
#include "utils/fmgrprotos.h" #include "utils/fmgrprotos.h"
...@@ -191,32 +205,71 @@ foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr) ...@@ -191,32 +205,71 @@ foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
print $pfh "extern Datum $s->{prosrc}(PG_FUNCTION_ARGS);\n"; print $pfh "extern Datum $s->{prosrc}(PG_FUNCTION_ARGS);\n";
} }
# Create the fmgr_builtins table # Create the fmgr_builtins table, collect data for fmgr_builtin_oid_index
print $tfh "\nconst FmgrBuiltin fmgr_builtins[] = {\n"; print $tfh "\nconst FmgrBuiltin fmgr_builtins[] = {\n";
my %bmap; my %bmap;
$bmap{'t'} = 'true'; $bmap{'t'} = 'true';
$bmap{'f'} = 'false'; $bmap{'f'} = 'false';
my @fmgr_builtin_oid_index;
my $fmgr_count = 0;
foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr) foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
{ {
print $tfh print $tfh
" { $s->{oid}, \"$s->{prosrc}\", $s->{nargs}, $bmap{$s->{strict}}, $bmap{$s->{retset}}, $s->{prosrc} },\n"; " { $s->{oid}, \"$s->{prosrc}\", $s->{nargs}, $bmap{$s->{strict}}, $bmap{$s->{retset}}, $s->{prosrc} }";
$fmgr_builtin_oid_index[$s->{oid}] = $fmgr_count++;
if ($fmgr_count <= $#fmgr)
{
print $tfh ",\n";
}
else
{
print $tfh "\n";
}
}
print $tfh "};\n";
print $tfh qq|
const int fmgr_nbuiltins = (sizeof(fmgr_builtins) / sizeof(FmgrBuiltin));
|;
# Create fmgr_builtins_oid_index table.
#
# Note that the array has to be filled up to FirstBootstrapObjectId,
# as we can't rely on zero initialization as 0 is a valid mapping.
print $tfh qq|
const uint16 fmgr_builtin_oid_index[FirstBootstrapObjectId] = {
|;
for (my $i = 0; $i < $FirstBootstrapObjectId; $i++)
{
my $oid = $fmgr_builtin_oid_index[$i];
# fmgr_builtin_oid_index is sparse, map nonexistant functions to
# InvalidOidBuiltinMapping
if (not defined $oid)
{
$oid = 'InvalidOidBuiltinMapping';
}
if ($i + 1 == $FirstBootstrapObjectId)
{
print $tfh " $oid\n";
}
else
{
print $tfh " $oid,\n";
}
} }
print $tfh "};\n";
# And add the file footers. # And add the file footers.
print $ofh "\n#endif /* FMGROIDS_H */\n"; print $ofh "\n#endif /* FMGROIDS_H */\n";
print $pfh "\n#endif /* FMGRPROTOS_H */\n"; print $pfh "\n#endif /* FMGRPROTOS_H */\n";
print $tfh
qq| /* dummy entry is easier than getting rid of comma after last real one */
/* (not that there has ever been anything wrong with *having* a
comma after the last field in an array initializer) */
{ 0, NULL, 0, false, false, NULL }
};
/* Note fmgr_nbuiltins excludes the dummy entry */
const int fmgr_nbuiltins = (sizeof(fmgr_builtins) / sizeof(FmgrBuiltin)) - 1;
|;
close($ofh); close($ofh);
close($pfh); close($pfh);
close($tfh); close($tfh);
......
...@@ -25,7 +25,7 @@ fmgrprotos.h: fmgroids.h ; ...@@ -25,7 +25,7 @@ fmgrprotos.h: fmgroids.h ;
fmgroids.h: fmgrtab.c ; fmgroids.h: fmgrtab.c ;
fmgrtab.c: Gen_fmgrtab.pl $(catalogdir)/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h fmgrtab.c: Gen_fmgrtab.pl $(catalogdir)/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
$(PERL) -I $(catalogdir) $< $(top_srcdir)/src/include/catalog/pg_proc.h $(PERL) -I $(catalogdir) $< -I $(top_srcdir)/src/include/ $(top_srcdir)/src/include/catalog/pg_proc.h
errcodes.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-errcodes.pl errcodes.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-errcodes.pl
$(PERL) $(srcdir)/generate-errcodes.pl $< > $@ $(PERL) $(srcdir)/generate-errcodes.pl $< > $@
......
...@@ -70,26 +70,21 @@ static Datum fmgr_security_definer(PG_FUNCTION_ARGS); ...@@ -70,26 +70,21 @@ static Datum fmgr_security_definer(PG_FUNCTION_ARGS);
static const FmgrBuiltin * static const FmgrBuiltin *
fmgr_isbuiltin(Oid id) fmgr_isbuiltin(Oid id)
{ {
int low = 0; uint16 index;
int high = fmgr_nbuiltins - 1;
/* fast lookup only possible if original oid still assigned */
if (id >= FirstBootstrapObjectId)
return NULL;
/* /*
* Loop invariant: low is the first index that could contain target entry, * Lookup function data. If there's a miss in that range it's likely a
* and high is the last index that could contain it. * nonexistant function, returning NULL here will trigger an ERROR later.
*/ */
while (low <= high) index = fmgr_builtin_oid_index[id];
{ if (index == InvalidOidBuiltinMapping)
int i = (high + low) / 2; return NULL;
const FmgrBuiltin *ptr = &fmgr_builtins[i];
return &fmgr_builtins[index];
if (id == ptr->foid)
return ptr;
else if (id > ptr->foid)
low = i + 1;
else
high = i - 1;
}
return NULL;
} }
/* /*
......
...@@ -13,13 +13,13 @@ ...@@ -13,13 +13,13 @@
#ifndef FMGRTAB_H #ifndef FMGRTAB_H
#define FMGRTAB_H #define FMGRTAB_H
#include "access/transam.h"
#include "fmgr.h" #include "fmgr.h"
/* /*
* This table stores info about all the built-in functions (ie, functions * This table stores info about all the built-in functions (ie, functions
* that are compiled into the Postgres executable). The table entries are * that are compiled into the Postgres executable).
* required to appear in Oid order, so that binary search can be used.
*/ */
typedef struct typedef struct
...@@ -36,4 +36,11 @@ extern const FmgrBuiltin fmgr_builtins[]; ...@@ -36,4 +36,11 @@ extern const FmgrBuiltin fmgr_builtins[];
extern const int fmgr_nbuiltins; /* number of entries in table */ extern const int fmgr_nbuiltins; /* number of entries in table */
/*
* Mapping from a builtin function's oid to the index in the fmgr_builtins
* array.
*/
#define InvalidOidBuiltinMapping UINT16_MAX
extern const uint16 fmgr_builtin_oid_index[FirstBootstrapObjectId];
#endif /* FMGRTAB_H */ #endif /* FMGRTAB_H */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment