On 18/09/2023 17:50, Matthias van de Meent wrote:
(initdb takes about 73ms locally with syncing disabled)

That's impressive. It takes about 600 ms on my laptop. Of which about 140 ms goes into processing the BKI file. And that's with "initdb -no-sync" option.

Various methods of reducing the size of postgres.bki were applied, as
detailed in the patch's commit message. I believe the current output
is still quite human readable.

Overall this does not seem very worthwhile to me.

One thing caught my eye though: We currently have an "open" command after every "create". Except for bootstrap relations; creating a bootstrap relation opens it implicitly. That seems like a weird inconsistency. If we make "create" to always open the relation, we can both make it more consistent and save a few lines. That's maybe worth doing, per the attached. It removes the "open" command altogether, as it's not needed anymore.

Looking at "perf" profile of initdb, I also noticed that a small but measurable amount of time is spent in the "isatty(0)" call in do_end(). Does anyone care about doing bootstrap mode interactively? We could probably remove that.

--
Heikki Linnakangas
Neon (https://neon.tech)
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index 81a1b7bfec..043ad9325f 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -99,7 +99,7 @@ static int num_columns_read = 0;
 /* NULLVAL is a reserved keyword */
 %token NULLVAL
 /* All the rest are unreserved, and should be handled in boot_ident! */
-%token <kw> OPEN XCLOSE XCREATE INSERT_TUPLE
+%token <kw> XCLOSE XCREATE INSERT_TUPLE
 %token <kw> XDECLARE INDEX ON USING XBUILD INDICES UNIQUE XTOAST
 %token <kw> OBJ_ID XBOOTSTRAP XSHARED_RELATION XROWTYPE_OID
 %token <kw> XFORCE XNOT XNULL
@@ -119,8 +119,7 @@ Boot_Queries:
 		;
 
 Boot_Query :
-		  Boot_OpenStmt
-		| Boot_CloseStmt
+		  Boot_CloseStmt
 		| Boot_CreateStmt
 		| Boot_InsertStmt
 		| Boot_DeclareIndexStmt
@@ -129,20 +128,11 @@ Boot_Query :
 		| Boot_BuildIndsStmt
 		;
 
-Boot_OpenStmt:
-		  OPEN boot_ident
-				{
-					do_start();
-					boot_openrel($2);
-					do_end();
-				}
-		;
-
 Boot_CloseStmt:
 		  XCLOSE boot_ident
 				{
 					do_start();
-					closerel($2);
+					boot_closerel($2);
 					do_end();
 				}
 		;
@@ -185,17 +175,17 @@ Boot_CreateStmt:
 					 */
 					mapped_relation = ($4 || shared_relation);
 
+					if (boot_reldesc)
+					{
+						elog(DEBUG4, "create: warning, open relation exists, closing first");
+						boot_closerel(NULL);
+					}
+
 					if ($4)
 					{
 						TransactionId relfrozenxid;
 						MultiXactId relminmxid;
 
-						if (boot_reldesc)
-						{
-							elog(DEBUG4, "create bootstrap: warning, open relation exists, closing first");
-							closerel(NULL);
-						}
-
 						boot_reldesc = heap_create($2,
 												   PG_CATALOG_NAMESPACE,
 												   shared_relation ? GLOBALTABLESPACE_OID : 0,
@@ -239,6 +229,7 @@ Boot_CreateStmt:
 													  InvalidOid,
 													  NULL);
 						elog(DEBUG4, "relation created with OID %u", id);
+						boot_openrel(id);
 					}
 					do_end();
 				}
@@ -466,7 +457,6 @@ boot_column_val:
 
 boot_ident:
 		  ID			{ $$ = $1; }
-		| OPEN			{ $$ = pstrdup($1); }
 		| XCLOSE		{ $$ = pstrdup($1); }
 		| XCREATE		{ $$ = pstrdup($1); }
 		| INSERT_TUPLE	{ $$ = pstrdup($1); }
diff --git a/src/backend/bootstrap/bootscanner.l b/src/backend/bootstrap/bootscanner.l
index 6a9d4193f2..efb9e36090 100644
--- a/src/backend/bootstrap/bootscanner.l
+++ b/src/backend/bootstrap/bootscanner.l
@@ -71,8 +71,6 @@ sid		\'([^']|\'\')*\'
 
 %%
 
-open			{ boot_yylval.kw = "open"; return OPEN; }
-
 close			{ boot_yylval.kw = "close"; return XCLOSE; }
 
 create			{ boot_yylval.kw = "create"; return XCREATE; }
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 5810f8825e..2fc01f9d4d 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -409,13 +409,10 @@ bootstrap_signals(void)
  * ----------------
  */
 void
-boot_openrel(char *relname)
+boot_openrel(Oid id)
 {
 	int			i;
 
-	if (strlen(relname) >= NAMEDATALEN)
-		relname[NAMEDATALEN - 1] = '\0';
-
 	/*
 	 * pg_type must be filled before any OPEN command is executed, hence we
 	 * can now populate Typ if we haven't yet.
@@ -424,12 +421,12 @@ boot_openrel(char *relname)
 		populate_typ_list();
 
 	if (boot_reldesc != NULL)
-		closerel(NULL);
+		boot_closerel(NULL);
 
-	elog(DEBUG4, "open relation %s, attrsize %d",
-		 relname, (int) ATTRIBUTE_FIXED_PART_SIZE);
+	elog(DEBUG4, "open relation %u, attrsize %d",
+		 id, (int) ATTRIBUTE_FIXED_PART_SIZE);
 
-	boot_reldesc = table_openrv(makeRangeVar(NULL, relname, -1), NoLock);
+	boot_reldesc = table_open(id, NoLock);
 	numattr = RelationGetNumberOfAttributes(boot_reldesc);
 	for (i = 0; i < numattr; i++)
 	{
@@ -450,11 +447,11 @@ boot_openrel(char *relname)
 }
 
 /* ----------------
- *		closerel
+ *		boot_closerel
  * ----------------
  */
 void
-closerel(char *relname)
+boot_closerel(char *relname)
 {
 	if (relname)
 	{
@@ -498,7 +495,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
 	if (boot_reldesc != NULL)
 	{
 		elog(WARNING, "no open relations allowed with CREATE command");
-		closerel(NULL);
+		boot_closerel(NULL);
 	}
 
 	if (attrtypes[attnum] == NULL)
@@ -687,7 +684,7 @@ static void
 cleanup(void)
 {
 	if (boot_reldesc != NULL)
-		closerel(NULL);
+		boot_closerel(NULL);
 }
 
 /* ----------------
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 380bc23c82..3f4bf2ecc2 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -533,13 +533,6 @@ EOM
 		print $def $line;
 	}
 
-	# Open it, unless it's a bootstrap catalog (create bootstrap does this
-	# automatically)
-	if (!$catalog->{bootstrap})
-	{
-		print $bki "open $catname\n";
-	}
-
 	# For pg_attribute.h, we generate data entries ourselves.
 	if ($catname eq 'pg_attribute')
 	{
diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h
index e1cb73c5f2..e7aa2ec0da 100644
--- a/src/include/bootstrap/bootstrap.h
+++ b/src/include/bootstrap/bootstrap.h
@@ -34,8 +34,8 @@ extern PGDLLIMPORT int numattr;
 
 extern void BootstrapModeMain(int argc, char *argv[], bool check_only) pg_attribute_noreturn();
 
-extern void closerel(char *relname);
-extern void boot_openrel(char *relname);
+extern void boot_closerel(char *relname);
+extern void boot_openrel(Oid id);
 
 extern void DefineAttr(char *name, char *type, int attnum, int nullness);
 extern void InsertOneTuple(void);

Reply via email to