public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matthias van de Meent <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: GenBKI emits useless open;close for catalogs without rows
Date: Fri, 1 Sep 2023 19:26:56 +0200
Message-ID: <CAEze2Wh+uwUxKiDhYRBKuxin8A3nSuKf20LbSZwdmm1VKj_TWg@mail.gmail.com> (raw)
Hi,
Whilst looking at PostgreSQL's bootstrapping process, I noticed that
postgres.bki contains quite a few occurrances of the pattern "open
$catname; close $catname".
I suppose this pattern isn't too expensive, but according to my
limited research a combined open+close cycle doens't do anything
meaningful, so it does waste some CPU cycles in the process.
The attached patch 1 removes the occurances of those combined
open/close statements in postgresql.bki. Locally it passes
check-world, so I assume that opening and closing a table is indeed
not required for initializing a data-less catalog during
bootstrapping.
A potential addition to the patch would to stop manually closing
relations: initdb and check-world succeed without manual 'close'
operations because the 'open' command auto-closes the previous open
relation (in boot_openrel). Testing also suggests that the last opened
relation apparently doesn't need closing - check-world succeeds
without issues (incl. with TAP enabled). That is therefore implemented
in attached patch 2 - it removes the 'close' syntax in its entirety.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
Attachments:
[application/octet-stream] 0002-Remove-the-bki-close-command.patch (3.2K, ../CAEze2Wh+uwUxKiDhYRBKuxin8A3nSuKf20LbSZwdmm1VKj_TWg@mail.gmail.com/2-0002-Remove-the-bki-close-command.patch)
download | inline diff:
From 65b21f740fc09dcf80ed60a77a3a64d85605a203 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <[email protected]>
Date: Fri, 1 Sep 2023 19:23:17 +0200
Subject: [PATCH 2/2] Remove the bki 'close' command.
Bootstrapping works without manually closing relations because they are
auto-closed when a different relation is opened, and apparently leaking
a single opened relation at the end of the runway isn't a problem in the
currently available tests.
---
src/backend/bootstrap/bootparse.y | 13 +------------
src/backend/bootstrap/bootscanner.l | 2 --
src/backend/catalog/genbki.pl | 12 ------------
3 files changed, 1 insertion(+), 26 deletions(-)
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index 81a1b7bfec..e405ed1c7b 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> OPEN 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
@@ -120,7 +120,6 @@ Boot_Queries:
Boot_Query :
Boot_OpenStmt
- | Boot_CloseStmt
| Boot_CreateStmt
| Boot_InsertStmt
| Boot_DeclareIndexStmt
@@ -138,15 +137,6 @@ Boot_OpenStmt:
}
;
-Boot_CloseStmt:
- XCLOSE boot_ident
- {
- do_start();
- closerel($2);
- do_end();
- }
- ;
-
Boot_CreateStmt:
XCREATE boot_ident oidspec optbootstrap optsharedrelation optrowtypeoid LPAREN
{
@@ -467,7 +457,6 @@ boot_column_val:
boot_ident:
ID { $$ = $1; }
| OPEN { $$ = pstrdup($1); }
- | XCLOSE { $$ = pstrdup($1); }
| XCREATE { $$ = pstrdup($1); }
| INSERT_TUPLE { $$ = pstrdup($1); }
| XDECLARE { $$ = pstrdup($1); }
diff --git a/src/backend/bootstrap/bootscanner.l b/src/backend/bootstrap/bootscanner.l
index 6a9d4193f2..99cb7861c9 100644
--- a/src/backend/bootstrap/bootscanner.l
+++ b/src/backend/bootstrap/bootscanner.l
@@ -73,8 +73,6 @@ sid \'([^']|\'\')*\'
open { boot_yylval.kw = "open"; return OPEN; }
-close { boot_yylval.kw = "close"; return XCLOSE; }
-
create { boot_yylval.kw = "create"; return XCREATE; }
OID { boot_yylval.kw = "OID"; return OBJ_ID; }
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 67d63864b3..814d945cb1 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -549,8 +549,6 @@ EOM
{
print $def $line;
}
-
- my $opened = 0;
# Open it, unless it's a bootstrap catalog (create bootstrap does this
# automatically)
@@ -559,13 +557,8 @@ EOM
if (($catalog_data{$catname} || 0) > 0 || $catname eq 'pg_attribute')
{
print $bki "open $catname\n";
- $opened = 1;
}
}
- else
- {
- $opened = 1;
- }
# For pg_attribute.h, we generate data entries ourselves.
if ($catname eq 'pg_attribute')
@@ -681,11 +674,6 @@ EOM
}
}
- if ($opened eq 1)
- {
- print $bki "close $catname\n";
- }
-
printf $def "\n#endif\t\t\t\t\t\t\t/* %s_D_H */\n", uc $catname;
# Close and rename definition header
--
2.40.1
[application/octet-stream] 0001-Stop-emitting-open-nodata-close-patterns-in-genbki.p.patch (1.3K, ../CAEze2Wh+uwUxKiDhYRBKuxin8A3nSuKf20LbSZwdmm1VKj_TWg@mail.gmail.com/3-0001-Stop-emitting-open-nodata-close-patterns-in-genbki.p.patch)
download | inline diff:
From 4410ff8859e4239467ba29bda8ded0fd4603d175 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <[email protected]>
Date: Wed, 11 Jan 2023 02:13:04 +0100
Subject: [PATCH 1/2] Stop emitting open/nodata/close patterns in genbki.pl
Although opening and immediately closing the relation is not that
expensive, it's still cheaper to not touch it at all.
---
src/backend/catalog/genbki.pl | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 4a7205472c..67d63864b3 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -549,12 +549,22 @@ EOM
{
print $def $line;
}
+
+ my $opened = 0;
# Open it, unless it's a bootstrap catalog (create bootstrap does this
# automatically)
if (!$catalog->{bootstrap})
{
- print $bki "open $catname\n";
+ if (($catalog_data{$catname} || 0) > 0 || $catname eq 'pg_attribute')
+ {
+ print $bki "open $catname\n";
+ $opened = 1;
+ }
+ }
+ else
+ {
+ $opened = 1;
}
# For pg_attribute.h, we generate data entries ourselves.
@@ -671,7 +681,11 @@ EOM
}
}
- print $bki "close $catname\n";
+ if ($opened eq 1)
+ {
+ print $bki "close $catname\n";
+ }
+
printf $def "\n#endif\t\t\t\t\t\t\t/* %s_D_H */\n", uc $catname;
# Close and rename definition header
--
2.40.1
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: GenBKI emits useless open;close for catalogs without rows
In-Reply-To: <CAEze2Wh+uwUxKiDhYRBKuxin8A3nSuKf20LbSZwdmm1VKj_TWg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox