From 11fc054fda92fafc8b6c1ac66f70ecf059c61a9d Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Tue, 26 Sep 2017 12:40:56 -0700
Subject: [PATCH] Speed up fmgr_isbuiltin() by keeping an oid -> builtin
 mapping.

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

To address that, have Gen_fmgrtab.pl build an oid -> fmgr_builtins
index mapping. With that mapping fmgr_isbuiltin() just consists out of
a comparison and two array lookups.

To avoid having to reference extern variables, move the generated
table to fmgrtab.h and include that from fmgr.c.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914065128.a5sk7z4xde5uy3ei@alap3.anarazel.de
---
 src/backend/Makefile             | 14 +++++--
 src/backend/utils/Gen_fmgrtab.pl | 82 ++++++++++++++++++++++++++++++----------
 src/backend/utils/Makefile       |  7 ++--
 src/backend/utils/fmgr/fmgr.c    | 27 ++++++-------
 src/include/fmgr.h               | 16 ++++++++
 src/include/utils/fmgrtab.h      | 39 -------------------
 6 files changed, 102 insertions(+), 83 deletions(-)
 delete mode 100644 src/include/utils/fmgrtab.h

diff --git a/src/backend/Makefile b/src/backend/Makefile
index aab676dbbd..f0eb5d8d78 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -142,6 +142,8 @@ utils/errcodes.h: utils/generate-errcodes.pl utils/errcodes.txt
 # see explanation in parser/Makefile
 utils/fmgrprotos.h: utils/fmgroids.h ;
 
+utils/fmgrtab.h: utils/fmgroids.h ;
+
 utils/fmgroids.h: utils/Gen_fmgrtab.pl catalog/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
 	$(MAKE) -C utils $(notdir $@)
 
@@ -169,7 +171,7 @@ submake-schemapg:
 
 .PHONY: generated-headers
 
-generated-headers: $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/fmgrprotos.h $(top_builddir)/src/include/utils/probes.h
+generated-headers: $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/fmgrprotos.h $(top_builddir)/src/include/utils/fmgrtab.h $(top_builddir)/src/include/utils/probes.h
 
 $(top_builddir)/src/include/parser/gram.h: parser/gram.h
 	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
@@ -201,6 +203,11 @@ $(top_builddir)/src/include/utils/fmgrprotos.h: utils/fmgrprotos.h
 	  cd '$(dir $@)' && rm -f $(notdir $@) && \
 	  $(LN_S) "$$prereqdir/$(notdir $<)" .
 
+$(top_builddir)/src/include/utils/fmgrtab.h: utils/fmgrtab.h
+	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
+	  cd '$(dir $@)' && rm -f $(notdir $@) && \
+	  $(LN_S) "$$prereqdir/$(notdir $<)" .
+
 $(top_builddir)/src/include/utils/probes.h: utils/probes.h
 	cd '$(dir $@)' && rm -f $(notdir $@) && \
 	    $(LN_S) "../../../$(subdir)/utils/probes.h" .
@@ -219,7 +226,7 @@ distprep:
 	$(MAKE) -C catalog	schemapg.h postgres.bki postgres.description postgres.shdescription
 	$(MAKE) -C replication	repl_gram.c repl_scanner.c syncrep_gram.c syncrep_scanner.c
 	$(MAKE) -C storage/lmgr	lwlocknames.h
-	$(MAKE) -C utils	fmgrtab.c fmgroids.h fmgrprotos.h errcodes.h
+	$(MAKE) -C utils	fmgroids.h fmgrprotos.h fmgrtab.h errcodes.h
 	$(MAKE) -C utils/misc	guc-file.c
 	$(MAKE) -C utils/sort	qsort_tuple.c
 
@@ -312,6 +319,7 @@ clean:
 		$(top_builddir)/src/include/storage/lwlocknames.h \
 		$(top_builddir)/src/include/utils/fmgroids.h \
 		$(top_builddir)/src/include/utils/fmgrprotos.h \
+		$(top_builddir)/src/include/utils/fmgrtab.h \
 		$(top_builddir)/src/include/utils/probes.h
 ifeq ($(PORTNAME), cygwin)
 	rm -f postgres.dll libpostgres.a
@@ -341,7 +349,7 @@ maintainer-clean: distclean
 	      storage/lmgr/lwlocknames.h \
 	      utils/fmgroids.h \
 	      utils/fmgrprotos.h \
-	      utils/fmgrtab.c \
+	      utils/fmgrtab.h \
 	      utils/errcodes.h \
 	      utils/misc/guc-file.c \
 	      utils/sort/qsort_tuple.c
diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index 17851fe2a4..c09f8dedb1 100644
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -2,7 +2,7 @@
 #-------------------------------------------------------------------------
 #
 # Gen_fmgrtab.pl
-#    Perl script that generates fmgroids.h and fmgrtab.c from pg_proc.h
+#    Perl script that generates fmgroids.h and fmgrtab.h from pg_proc.h
 #
 # Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
 # Portions Copyright (c) 1994, Regents of the University of California
@@ -79,7 +79,7 @@ foreach my $row (@$data)
 my $tmpext     = ".tmp$$";
 my $oidsfile   = $output_path . 'fmgroids.h';
 my $protosfile = $output_path . 'fmgrprotos.h';
-my $tabfile    = $output_path . 'fmgrtab.c';
+my $tabfile    = $output_path . 'fmgrtab.h';
 
 open my $ofh, '>', $oidsfile . $tmpext
   or die "Could not open $oidsfile$tmpext: $!";
@@ -156,9 +156,12 @@ qq|/*-------------------------------------------------------------------------
 print $tfh
 qq|/*-------------------------------------------------------------------------
  *
- * fmgrtab.c
+ * fmgrtab.h
  *    The function manager's table of internal functions.
  *
+ * This should only be included once, by fmgr.c. Therefore no include guards
+ * are present.
+ *
  * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
@@ -174,10 +177,7 @@ qq|/*-------------------------------------------------------------------------
  *-------------------------------------------------------------------------
  */
 
-#include "postgres.h"
-
-#include "utils/fmgrtab.h"
-#include "utils/fmgrprotos.h"
+#include "fmgr.h"
 
 |;
 
@@ -191,32 +191,72 @@ foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
 	print $pfh "extern Datum $s->{prosrc}(PG_FUNCTION_ARGS);\n";
 }
 
-# Create the fmgr_builtins table
-print $tfh "\nconst FmgrBuiltin fmgr_builtins[] = {\n";
+# Create the fmgr_builtins table, collect data for fmgr_builtin_oid_index
+print $tfh "\nstatic const 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|
+static const int fmgr_nbuiltins = (sizeof(fmgr_builtins) / sizeof(FmgrBuiltin));
+
+static const Oid fmgr_builtin_max_oid = $fmgr_builtin_max_oid;
+
+|;
+
+# Create fmgr_builtins_oid_index table.
+print $tfh "static const uint16 fmgr_builtin_oid_index[] = {\n";
+for (my $i = 0; $i <= $fmgr_builtin_max_oid; $i++)
+{
+    my $oid = $fmgr_builtin_oid_index[$i];
+
+    # fmgr_builtin_oid_index is sparse, map nonexistant functions to 0
+    if (not defined $oid)
+    {
+		$oid = 0;
+    }
+
+    if ($i == $fmgr_builtin_max_oid)
+    {
+		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/Makefile b/src/backend/utils/Makefile
index 2e35ca58cc..b299d1fa45 100644
--- a/src/backend/utils/Makefile
+++ b/src/backend/utils/Makefile
@@ -8,7 +8,6 @@ subdir = src/backend/utils
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS        = fmgrtab.o
 SUBDIRS     = adt cache error fmgr hash init mb misc mmgr resowner sort time
 
 # location of Catalog.pm
@@ -22,9 +21,9 @@ $(SUBDIRS:%=%-recursive): fmgroids.h fmgrprotos.h
 
 # see explanation in ../parser/Makefile
 fmgrprotos.h: fmgroids.h ;
-fmgroids.h: fmgrtab.c ;
+fmgroids.h: fmgrtab.h ;
 
-fmgrtab.c: Gen_fmgrtab.pl $(catalogdir)/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
+fmgrtab.h: 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
 
 errcodes.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-errcodes.pl
@@ -50,4 +49,4 @@ clean:
 	rm -f probes.h
 
 maintainer-clean: clean
-	rm -f fmgroids.h fmgrprotos.h fmgrtab.c errcodes.h
+	rm -f fmgroids.h fmgrprotos.h fmgrtab.h errcodes.h
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index a7b07827e0..4fba2514f0 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;
+	uint16		index;
+
+	/* fast lookup only possible if original oid still assigned */
+	if (id >= fmgr_builtin_max_oid)
+		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 == 0)
+		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/fmgr.h b/src/include/fmgr.h
index b604a5c162..ff79ee3dd5 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -729,6 +729,22 @@ extern PGDLLIMPORT fmgr_hook_type fmgr_hook;
 #define FmgrHookIsNeeded(fn_oid)							\
 	(!needs_fmgr_hook ? false : (*needs_fmgr_hook)(fn_oid))
 
+
+/*
+ * Description of all the built-in functions (ie, functions that are compiled
+ * into the Postgres executable).
+ */
+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 */
+} FmgrBuiltin;
+
+
 /*
  * !!! OLD INTERFACE !!!
  *
diff --git a/src/include/utils/fmgrtab.h b/src/include/utils/fmgrtab.h
deleted file mode 100644
index 6130ef8f9c..0000000000
--- a/src/include/utils/fmgrtab.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * fmgrtab.h
- *	  The function manager's table of internal functions.
- *
- * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- * src/include/utils/fmgrtab.h
- *
- *-------------------------------------------------------------------------
- */
-#ifndef FMGRTAB_H
-#define FMGRTAB_H
-
-#include "fmgr.h"
-
-
-/*
- * This table stores info about all the built-in functions (ie, functions
- * that are compiled into the Postgres executable).  The table entries are
- * required to appear in Oid order, so that binary search can be used.
- */
-
-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 */
-} FmgrBuiltin;
-
-extern const FmgrBuiltin fmgr_builtins[];
-
-extern const int fmgr_nbuiltins;	/* number of entries in table */
-
-#endif							/* FMGRTAB_H */
-- 
2.14.1.536.g6867272d5b.dirty

