From 2dfe8a94a8bea160e9cac512f0d3346fdc90cf44 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 2 Oct 2017 14:27:05 -0700
Subject: [PATCH 2/2] 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
Discussion: https://postgr.es/m/20170914065128.a5sk7z4xde5uy3ei@alap3.anarazel.de
---
 src/backend/utils/Gen_fmgrtab.pl | 84 +++++++++++++++++++++++++++++++++-------
 src/backend/utils/fmgr/fmgr.c    | 27 ++++++-------
 src/include/utils/fmgrtab.h      |  8 ++++
 3 files changed, 90 insertions(+), 29 deletions(-)

diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index 17851fe2a4..b8821798e6 100644
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -21,6 +21,8 @@ use warnings;
 # Collect arguments
 my $infile;    # pg_proc.h
 my $output_path = '';
+my @include_path;
+
 while (@ARGV)
 {
 	my $arg = shift @ARGV;
@@ -32,6 +34,10 @@ while (@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
 	{
 		usage();
@@ -44,6 +50,13 @@ if ($output_path ne '' && substr($output_path, -1) ne '/')
 	$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.
 my $catalogs = Catalog::Catalogs($infile);
 
@@ -176,6 +189,7 @@ qq|/*-------------------------------------------------------------------------
 
 #include "postgres.h"
 
+#include "access/transam.h"
 #include "utils/fmgrtab.h"
 #include "utils/fmgrprotos.h"
 
@@ -191,32 +205,76 @@ foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
 	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";
 my %bmap;
 $bmap{'t'} = 'true';
 $bmap{'f'} = 'false';
+my $fmgr_builtin_max_oid = -1;
+my @fmgr_builtin_oid_index;
+my $fmgr_count = 0;
 foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
 {
 	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} }";
+
+	if ($s->{oid} > $fmgr_builtin_max_oid)
+	{
+		$fmgr_builtin_max_oid = $s->{oid};
+	}
+
+	$fmgr_builtin_oid_index[$s->{oid}] = $fmgr_count++;
+
+	if ($fmgr_count <= $#fmgr)
+	{
+		print $tfh ",\n";
+	}
+	else
+	{
+		print $tfh "\n";
+	}
 }
+print $tfh "};";
+
+print $tfh qq|
+const int fmgr_nbuiltins = (sizeof(fmgr_builtins) / sizeof(FmgrBuiltin));
+
+const Oid fmgr_builtin_max_oid = $fmgr_builtin_max_oid;
+
+|;
+
+# 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 "const uint16 fmgr_builtin_oid_index[FirstBootstrapObjectId] = {\n";
+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 "};";
+
 
 # And add the file footers.
 print $ofh "\n#endif /* FMGROIDS_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($pfh);
 close($tfh);
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 919733517b..971ea40bee 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -70,26 +70,21 @@ static Datum fmgr_security_definer(PG_FUNCTION_ARGS);
 static const FmgrBuiltin *
 fmgr_isbuiltin(Oid id)
 {
-	int			low = 0;
-	int			high = fmgr_nbuiltins - 1;
+	int16		index;
+
+	/* 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,
-	 * and high is the last index that could contain it.
+	 * Lookup function data. If there's a miss in that range it's likely a
+	 * nonexistant function, returning NULL here will trigger an ERROR later.
 	 */
-	while (low <= high)
-	{
-		int			i = (high + low) / 2;
-		const FmgrBuiltin *ptr = &fmgr_builtins[i];
+	index = fmgr_builtin_oid_index[id];
+	if (index == InvalidOidBuiltinMapping)
+		return NULL;
 
-		if (id == ptr->foid)
-			return ptr;
-		else if (id > ptr->foid)
-			low = i + 1;
-		else
-			high = i - 1;
-	}
-	return NULL;
+	return &fmgr_builtins[index];
 }
 
 /*
diff --git a/src/include/utils/fmgrtab.h b/src/include/utils/fmgrtab.h
index 6130ef8f9c..dbd2c0b8e1 100644
--- a/src/include/utils/fmgrtab.h
+++ b/src/include/utils/fmgrtab.h
@@ -13,6 +13,7 @@
 #ifndef FMGRTAB_H
 #define FMGRTAB_H
 
+#include "access/transam.h"
 #include "fmgr.h"
 
 
@@ -36,4 +37,11 @@ extern const FmgrBuiltin fmgr_builtins[];
 
 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 -1
+extern const uint16 fmgr_builtin_oid_index[FirstBootstrapObjectId];
+
 #endif							/* FMGRTAB_H */
-- 
2.14.1.536.g6867272d5b.dirty

