public inbox for [email protected]
help / color / mirror / Atom feedRe: meson documentation build open issues
27+ messages / 3 participants
[nested] [flat]
* Re: meson documentation build open issues
@ 2023-11-08 23:21 Andres Freund <[email protected]>
2023-11-09 14:39 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 27+ messages in thread
From: Andres Freund @ 2023-11-08 23:21 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
I really like the idea of an 'help' target that prints the targets. It seemed
annoying to document such targets in both the sgml docs and the input for a
the help target. Particularly due to the redundancies between id attributes,
the target name etc.
First I generated the list of targets from within meson.build, only to later
realize that that would not work when building the docs via make. So I instead
added doc/src/sgml/meson-targets.txt which is lightly postprocessed for the
'help' target, and slightly more processed when building the docs.
That does have some downsides, e.g. it'd be more complicated to only print
targets if a relevant option is enabled. But I think it's acceptable that way.
Example output:
$ ninja help
[0/1 1 0%] Running external command help (wrapped by meson to set env)
Code Targets:
all Build everything other than documentation
backend Build backend and related modules
bin Build frontend binaries
contrib Build contrib modules
pl Build procedual languages
Documentation Targets:
docs Build documentation in multi-page HTML format
doc-html Build documentation in multi-page HTML format
doc-man Build documentation in man page format
doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with A4 pages
doc/src/sgml/postgres-US.pdf Build documentation in PDF format, with US letter pages
doc/src/sgml/postgres.html Build documentation in single-page HTML format
alldocs Build documentation in all supported formats
Installation Targets:
install Install postgres, excluding documentation
install-doc-html Install documentation in multi-page HTML format
install-doc-man Install documentation in man page format
install-docs Install documentation in multi-page HTML and man page formats
install-quiet Like "install", but installed files are not displayed
install-world Install postgres, including multi-page HTML and man page documentation
uninstall Remove installed files
Other Targets:
clean Remove all build products
test Run all enabled tests (including contrib)
world Build everything, including documentation
help List important targets
Because of the common source, some of the descriptions in the state of this
patch are a bit shorter than in the preceding commit. But I don't think that
hurts much.
Greetings,
Andres Freund
Attachments:
[text/x-diff] v2-0001-meson-Change-default-of-selinux-feature-option-to.patch (1.0K, ../../[email protected]/2-v2-0001-meson-Change-default-of-selinux-feature-option-to.patch)
download | inline diff:
From 9b0b5cd952880ecebbd157c05698125755bc53ed Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Wed, 8 Nov 2023 09:29:38 -0800
Subject: [PATCH v2 1/7] meson: Change default of 'selinux' feature option to
auto
There is really no reason for selinux to behave differently than other
options.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
meson_options.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson_options.txt b/meson_options.txt
index d2f95cfec36..be1b327f544 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -136,7 +136,7 @@ option('tcl_version', type: 'string', value: 'tcl',
option('readline', type: 'feature', value: 'auto',
description: 'Use GNU Readline or BSD Libedit for editing')
-option('selinux', type: 'feature', value: 'disabled',
+option('selinux', type: 'feature', value: 'auto',
description: 'SELinux support')
option('ssl', type: 'combo', choices: ['auto', 'none', 'openssl'],
--
2.38.0
[text/x-diff] v2-0002-docs-Document-with-selinux-Dselinux-options-centr.patch (2.7K, ../../[email protected]/3-v2-0002-docs-Document-with-selinux-Dselinux-options-centr.patch)
download | inline diff:
From a2aa43811296c92a8d4611c996d4aebcf1b88f31 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 3 Nov 2023 10:21:13 -0700
Subject: [PATCH v2 2/7] docs: Document --with-selinux/-Dselinux options
centrally
Previously --with-selinux was documented for autoconf in the sepgsql
documentation and not at all for meson. There are further improvements related
to this that could be made, but this seems like a clear improvement.
Author:
Reviewed-by:
Reported-by: Christoph Berg <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
doc/src/sgml/installation.sgml | 21 +++++++++++++++++++++
doc/src/sgml/sepgsql.sgml | 11 ++++++++---
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index a3dc6eb855f..1bfb27fd38d 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1198,6 +1198,16 @@ build-postgresql:
</listitem>
</varlistentry>
+ <varlistentry id="configure-option-with-sepgsql">
+ <term><option>--with-selinux</option></term>
+ <listitem>
+ <para>
+ Build with selinux support, enabling the <xref linkend="sepgsql"/>
+ extension.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect3>
@@ -2629,6 +2639,17 @@ ninja install
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="configure-with-sepgsql-meson">
+ <term><option>-Dselinux={ auto | enabled | disabled }</option></term>
+ <listitem>
+ <para>
+ Build with selinux support, enabling the <xref linkend="sepgsql"/>
+ extension. Defaults to auto.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect3>
diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index b368e587cbf..1b848f1977c 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -87,9 +87,14 @@ Policy from config file: targeted
</para>
<para>
- To build this module, include the option <literal>--with-selinux</literal> in
- your PostgreSQL <literal>configure</literal> command. Be sure that the
- <filename>libselinux-devel</filename> RPM is installed at build time.
+ To build this module specify <xref
+ linkend="configure-option-with-sepgsql"/> (when using <link
+ linkend="install-make">make and autoconf</link> ) or <xref
+ linkend="configure-with-sepgsql-meson"/> (when using <link
+ linkend="install-meson">meson</link>).
+
+ Be sure that the <filename>libselinux-devel</filename> RPM is installed at
+ build time.
</para>
<para>
--
2.38.0
[text/x-diff] v2-0003-meson-docs-Add-doc-html-man-targets.patch (1.2K, ../../[email protected]/4-v2-0003-meson-docs-Add-doc-html-man-targets.patch)
download | inline diff:
From a8dc88cf6247a07b02dbef61e9ae7ef30af50131 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 3 Nov 2023 11:59:25 -0700
Subject: [PATCH v2 3/7] meson: docs: Add doc-{html,man} targets
Reviewed-by: Christoph Berg <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/meson.build | 3 +++
1 file changed, 3 insertions(+)
diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index 90e2c062fa8..fac7e701610 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -142,6 +142,7 @@ if docs_dep.found()
'--install-dir-contents', dir_doc_html, html],
build_always_stale: true, build_by_default: false,
)
+ alias_target('doc-html', html)
alias_target('install-doc-html', install_doc_html)
# build and install multi-page html docs as part of docs target
@@ -231,6 +232,8 @@ if docs_dep.found()
'--install-dirs', dir_man, '@INPUT@'],
build_always_stale: true, build_by_default: false,
)
+
+ alias_target('doc-man', man)
alias_target('install-doc-man', install_doc_man)
# even though we don't want to build man pages as part of 'docs', we do want
--
2.38.0
[text/x-diff] v2-0004-meson-Add-world-target.patch (739B, ../../[email protected]/5-v2-0004-meson-Add-world-target.patch)
download | inline diff:
From 65a248330069d2891ae48912fa2c947217f0c366 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Wed, 8 Nov 2023 12:39:05 -0800
Subject: [PATCH v2 4/7] meson: Add 'world' target
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
meson.build | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meson.build b/meson.build
index 88a9d9051fe..5b03e90c3db 100644
--- a/meson.build
+++ b/meson.build
@@ -3326,6 +3326,8 @@ alias_target('bin', bin_targets + [libpq_st])
alias_target('pl', pl_targets)
alias_target('contrib', contrib_targets)
alias_target('testprep', testprep_targets)
+
+alias_target('world', all_built, docs)
alias_target('install-world', install_quiet, installdocs)
--
2.38.0
[text/x-diff] v2-0005-docs-meson-Add-documentation-for-important-build-.patch (7.3K, ../../[email protected]/6-v2-0005-docs-meson-Add-documentation-for-important-build-.patch)
download | inline diff:
From e8bea2e38093371fad3af9ba65436696d4d01cca Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 3 Nov 2023 14:06:25 -0700
Subject: [PATCH v2 5/7] docs: meson: Add documentation for important build
targets
Author:
Reviewed-by: Christoph Berg <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
doc/src/sgml/installation.sgml | 244 +++++++++++++++++++++++++++++++++
1 file changed, 244 insertions(+)
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 1bfb27fd38d..9dde19224a7 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -3200,6 +3200,250 @@ ninja install
</variablelist>
</sect3>
</sect2>
+
+ <sect2 id="meson-targets">
+ <title><literal>meson</literal> Build Targets</title>
+
+ <para>
+ Individual build targets can be built using <command>ninja</command> <replaceable>target</replaceable>.
+
+ When no target is specified, everything except documentation is
+ built. Individual build products can be built using the path/filename as
+ <replaceable>target</replaceable>.
+ </para>
+
+ <sect3 id="meson-targets-code">
+ <title>Code Targets</title>
+
+ <variablelist>
+
+ <varlistentry id="meson-target-all">
+ <term><option>all</option></term>
+ <listitem>
+ <para>
+ Build everything other than documentation
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-backend">
+ <term><option>backend</option></term>
+ <listitem>
+ <para>
+ Build backend and related modules.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-bin">
+ <term><option>bin</option></term>
+ <listitem>
+ <para>
+ Build frontend binaries.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-contrib">
+ <term><option>contrib</option></term>
+ <listitem>
+ <para>
+ Build contrib modules.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-pl">
+ <term><option>pl</option></term>
+ <listitem>
+ <para>
+ Build procedual languages.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ </sect3>
+
+ <sect3 id="meson-targets-install">
+ <title>Install Targets</title>
+
+ <variablelist>
+
+ <varlistentry id="meson-target-install">
+ <term><option>install</option></term>
+ <listitem>
+ <para>
+ Install postgres, excluding documentation.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-install-quiet">
+ <term><option>install-quiet</option></term>
+ <listitem>
+ <para>
+ Like <xref linkend="meson-target-install"/>, but installed
+ files are not displayed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-install-world">
+ <term><option>install-world</option></term>
+ <listitem>
+ <para>
+ Install postgres, including multi-page HTML and man page
+ documentation.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-install-docs">
+ <term><option>install-docs</option></term>
+ <listitem>
+ <para>
+ Install documentation in multi-page HTML and man page formats. See
+ also <xref linkend="meson-target-install-doc-html"/>, <xref
+ linkend="meson-target-install-doc-man"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-install-doc-html">
+ <term><option>install-doc-html</option></term>
+ <listitem>
+ <para>
+ Install documentation in multi-page HTML format.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-install-doc-man">
+ <term><option>install-doc-man</option></term>
+ <listitem>
+ <para>
+ Install documentation in man page format.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-uninstall">
+ <term><option>uninstall</option></term>
+ <listitem>
+ <para>
+ Remove installed files.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+ </sect3>
+
+ <sect3 id="meson-targets-docs">
+ <title>Documentation Targets</title>
+
+ <variablelist>
+
+ <varlistentry id="meson-target-docs">
+ <term><option>docs</option></term>
+ <term><option>doc-html</option></term>
+ <listitem>
+ <para>
+ Build documentation in multi-page HTML format. Note that
+ <option>docs</option> does <emphasis>not</emphasis> include building
+ man page documentation, as man page generation seldom fails when
+ building HTML documentation succeeds.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-doc-man">
+ <term><option>doc-man</option></term>
+ <listitem>
+ <para>
+ Build documentation in man page format.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-html-single-page">
+ <term><option>doc/src/sgml/postgres.html</option></term>
+ <listitem>
+ <para>
+ Build documentation in single-page HTML format.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-pdf">
+ <term><option>doc/src/sgml/postgres-A4.pdf</option></term>
+ <term><option>doc/src/sgml/postgres-US.pdf</option></term>
+ <listitem>
+ <para>
+ Build documentation in PDF format, using A4 and U.S. letter format
+ respectively.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-alldocs">
+ <term><option>alldocs</option></term>
+ <listitem>
+ <para>
+ Build documentation in all supported formats. Primarily useful when
+ testing larger documentation changes.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ </sect3>
+
+ <sect3 id="meson-targets-other">
+ <title>Other Targets</title>
+
+ <variablelist>
+
+ <varlistentry id="meson-target-clean">
+ <term><option>clean</option></term>
+ <listitem>
+ <para>
+ Remove all build products
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="meson-target-test">
+ <term><option>test</option></term>
+ <listitem>
+ <para>
+ Run all enabled tests (including contrib). Support for some classes
+ of tests can be enabled / disabled with <xref
+ linkend="configure-tap-tests-meson"/> and <xref
+ linkend="configure-pg-test-extra-meson"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+
+ <varlistentry id="meson-target-world">
+ <term><option>world</option></term>
+ <listitem>
+ <para>
+ Build everything, including documentation.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ </sect3>
+
+ </sect2>
+
</sect1>
<sect1 id="install-post">
--
2.38.0
[text/x-diff] v2-0006-meson-Add-help-target-build-docs-from-a-common-so.patch (13.2K, ../../[email protected]/7-v2-0006-meson-Add-help-target-build-docs-from-a-common-so.patch)
download | inline diff:
From 619fb4f7b2289bbf2bda9156d866b89451cac70e Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Wed, 8 Nov 2023 15:03:00 -0800
Subject: [PATCH v2 6/7] meson: Add 'help' target, build docs from a common
source file
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
meson.build | 7 +
doc/src/sgml/Makefile | 3 +
doc/src/sgml/filelist.sgml | 1 +
doc/src/sgml/generate-meson-targets.pl | 63 +++++++
doc/src/sgml/installation.sgml | 231 +------------------------
doc/src/sgml/meson-targets.txt | 38 ++++
doc/src/sgml/meson.build | 9 +
7 files changed, 122 insertions(+), 230 deletions(-)
create mode 100644 doc/src/sgml/generate-meson-targets.pl
create mode 100644 doc/src/sgml/meson-targets.txt
diff --git a/meson.build b/meson.build
index 5b03e90c3db..0f086757e17 100644
--- a/meson.build
+++ b/meson.build
@@ -3330,6 +3330,13 @@ alias_target('testprep', testprep_targets)
alias_target('world', all_built, docs)
alias_target('install-world', install_quiet, installdocs)
+run_target('help',
+ command: [
+ perl, '-ne', 'next if /^#/; print',
+ files('doc/src/sgml/meson-targets.txt'),
+ ]
+)
+
###############################################################
diff --git a/doc/src/sgml/Makefile b/doc/src/sgml/Makefile
index 49d35dd0d6e..28858f42228 100644
--- a/doc/src/sgml/Makefile
+++ b/doc/src/sgml/Makefile
@@ -110,6 +110,9 @@ keywords-table.sgml: $(top_srcdir)/src/include/parser/kwlist.h $(wildcard $(srcd
wait_event_types.sgml: $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt $(top_srcdir)/src/backend/utils/activity/generate-wait_event_types.pl
$(PERL) $(top_srcdir)/src/backend/utils/activity/generate-wait_event_types.pl --docs $<
+meson-targets.sgml: meson-targets.txt $(srcdir)/generate-meson-targets.pl
+ $(PERL) $(srcdir)/generate-meson-targets.pl $^ > $@
+
##
## Generation of some text files.
##
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index 4c63a7e7689..ed89e75b614 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -38,6 +38,7 @@
<!ENTITY high-availability SYSTEM "high-availability.sgml">
<!ENTITY installbin SYSTEM "install-binaries.sgml">
<!ENTITY installation SYSTEM "installation.sgml">
+<!ENTITY meson-targets SYSTEM "meson-targets.sgml">
<!ENTITY installw SYSTEM "install-windows.sgml">
<!ENTITY maintenance SYSTEM "maintenance.sgml">
<!ENTITY manage-ag SYSTEM "manage-ag.sgml">
diff --git a/doc/src/sgml/generate-meson-targets.pl b/doc/src/sgml/generate-meson-targets.pl
new file mode 100644
index 00000000000..65314aee282
--- /dev/null
+++ b/doc/src/sgml/generate-meson-targets.pl
@@ -0,0 +1,63 @@
+#!/usr/bin/perl
+#
+# Generate the meson-targets.sgml file from meson-targets.txt
+# Copyright (c) 2000-2023, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+my $meson_targets_file = $ARGV[0];
+open my $meson_targets, '<', $meson_targets_file or die;
+
+print
+ "<!-- autogenerated from doc/src/sgml/meson-targets.txt, do not edit -->\n";
+
+# Find the start of each group of targets
+while (<$meson_targets>)
+{
+ next if /^#/;
+
+ if (/^(.*) Targets:$/)
+ {
+ my $targets = $1;
+ my $targets_id = lc $targets;
+
+ print qq(
+<sect3 id="meson-targets-$targets_id">
+ <title>$targets Targets</title>
+
+ <variablelist>
+);
+
+ # Each target in the group
+ while (<$meson_targets>)
+ {
+ next if /^#/;
+ last if !/^\s+([^ ]+)\s+(.+)/;
+
+ my $target = $1;
+ my $desc = $2;
+ my $target_id = $1;
+
+ $target_id =~ s/\//-/g;
+
+ print qq(
+ <varlistentry id="meson-target-${target_id}">
+ <term><option>${target}</option></term>
+ <listitem>
+ <para>
+ ${desc}
+ </para>
+ </listitem>
+ </varlistentry>
+);
+ }
+
+ print qq(
+ </variablelist>
+</sect3>
+);
+ }
+}
+
+close $meson_targets;
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 9dde19224a7..d7fefa0918c 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -3212,236 +3212,7 @@ ninja install
<replaceable>target</replaceable>.
</para>
- <sect3 id="meson-targets-code">
- <title>Code Targets</title>
-
- <variablelist>
-
- <varlistentry id="meson-target-all">
- <term><option>all</option></term>
- <listitem>
- <para>
- Build everything other than documentation
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-backend">
- <term><option>backend</option></term>
- <listitem>
- <para>
- Build backend and related modules.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-bin">
- <term><option>bin</option></term>
- <listitem>
- <para>
- Build frontend binaries.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-contrib">
- <term><option>contrib</option></term>
- <listitem>
- <para>
- Build contrib modules.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-pl">
- <term><option>pl</option></term>
- <listitem>
- <para>
- Build procedual languages.
- </para>
- </listitem>
- </varlistentry>
-
- </variablelist>
-
- </sect3>
-
- <sect3 id="meson-targets-install">
- <title>Install Targets</title>
-
- <variablelist>
-
- <varlistentry id="meson-target-install">
- <term><option>install</option></term>
- <listitem>
- <para>
- Install postgres, excluding documentation.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-install-quiet">
- <term><option>install-quiet</option></term>
- <listitem>
- <para>
- Like <xref linkend="meson-target-install"/>, but installed
- files are not displayed.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-install-world">
- <term><option>install-world</option></term>
- <listitem>
- <para>
- Install postgres, including multi-page HTML and man page
- documentation.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-install-docs">
- <term><option>install-docs</option></term>
- <listitem>
- <para>
- Install documentation in multi-page HTML and man page formats. See
- also <xref linkend="meson-target-install-doc-html"/>, <xref
- linkend="meson-target-install-doc-man"/>.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-install-doc-html">
- <term><option>install-doc-html</option></term>
- <listitem>
- <para>
- Install documentation in multi-page HTML format.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-install-doc-man">
- <term><option>install-doc-man</option></term>
- <listitem>
- <para>
- Install documentation in man page format.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-uninstall">
- <term><option>uninstall</option></term>
- <listitem>
- <para>
- Remove installed files.
- </para>
- </listitem>
- </varlistentry>
-
- </variablelist>
- </sect3>
-
- <sect3 id="meson-targets-docs">
- <title>Documentation Targets</title>
-
- <variablelist>
-
- <varlistentry id="meson-target-docs">
- <term><option>docs</option></term>
- <term><option>doc-html</option></term>
- <listitem>
- <para>
- Build documentation in multi-page HTML format. Note that
- <option>docs</option> does <emphasis>not</emphasis> include building
- man page documentation, as man page generation seldom fails when
- building HTML documentation succeeds.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-doc-man">
- <term><option>doc-man</option></term>
- <listitem>
- <para>
- Build documentation in man page format.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-html-single-page">
- <term><option>doc/src/sgml/postgres.html</option></term>
- <listitem>
- <para>
- Build documentation in single-page HTML format.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-pdf">
- <term><option>doc/src/sgml/postgres-A4.pdf</option></term>
- <term><option>doc/src/sgml/postgres-US.pdf</option></term>
- <listitem>
- <para>
- Build documentation in PDF format, using A4 and U.S. letter format
- respectively.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-alldocs">
- <term><option>alldocs</option></term>
- <listitem>
- <para>
- Build documentation in all supported formats. Primarily useful when
- testing larger documentation changes.
- </para>
- </listitem>
- </varlistentry>
-
- </variablelist>
-
- </sect3>
-
- <sect3 id="meson-targets-other">
- <title>Other Targets</title>
-
- <variablelist>
-
- <varlistentry id="meson-target-clean">
- <term><option>clean</option></term>
- <listitem>
- <para>
- Remove all build products
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="meson-target-test">
- <term><option>test</option></term>
- <listitem>
- <para>
- Run all enabled tests (including contrib). Support for some classes
- of tests can be enabled / disabled with <xref
- linkend="configure-tap-tests-meson"/> and <xref
- linkend="configure-pg-test-extra-meson"/>.
- </para>
- </listitem>
- </varlistentry>
-
-
- <varlistentry id="meson-target-world">
- <term><option>world</option></term>
- <listitem>
- <para>
- Build everything, including documentation.
- </para>
- </listitem>
- </varlistentry>
-
- </variablelist>
-
- </sect3>
-
+ &meson-targets;
</sect2>
</sect1>
diff --git a/doc/src/sgml/meson-targets.txt b/doc/src/sgml/meson-targets.txt
new file mode 100644
index 00000000000..690e4ebc814
--- /dev/null
+++ b/doc/src/sgml/meson-targets.txt
@@ -0,0 +1,38 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+#
+# Description of important meson targets, used for the 'help' target and
+# installation.sgml (via generate-meson-targets.pl). Right now the parsers are
+# extremely simple. Both parsers ignore comments. The help target prints
+# everything else. For xml everything without a leading newline is a group,
+# remaining lines are target separated by whitespace from their description
+#
+Code Targets:
+ all Build everything other than documentation
+ backend Build backend and related modules
+ bin Build frontend binaries
+ contrib Build contrib modules
+ pl Build procedual languages
+
+Documentation Targets:
+ docs Build documentation in multi-page HTML format
+ doc-html Build documentation in multi-page HTML format
+ doc-man Build documentation in man page format
+ doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with A4 pages
+ doc/src/sgml/postgres-US.pdf Build documentation in PDF format, with US letter pages
+ doc/src/sgml/postgres.html Build documentation in single-page HTML format
+ alldocs Build documentation in all supported formats
+
+Installation Targets:
+ install Install postgres, excluding documentation
+ install-doc-html Install documentation in multi-page HTML format
+ install-doc-man Install documentation in man page format
+ install-docs Install documentation in multi-page HTML and man page formats
+ install-quiet Like "install", but installed files are not displayed
+ install-world Install postgres, including multi-page HTML and man page documentation
+ uninstall Remove installed files
+
+Other Targets:
+ clean Remove all build products
+ test Run all enabled tests (including contrib)
+ world Build everything, including documentation
+ help List important targets
diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index fac7e701610..2dca38ccf7b 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -71,6 +71,15 @@ doc_generated += custom_target('keywords-table.sgml',
capture: true,
)
+doc_generated += custom_target('meson-targets.sgml',
+ input: files('meson-targets.txt'),
+ output: 'meson-targets.sgml',
+ command: [perl, files('generate-meson-targets.pl'), '@INPUT@'],
+ build_by_default: false,
+ install: false,
+ capture: true,
+)
+
# For everything else we need at least xmllint
if not xmllint_bin.found()
subdir_done()
--
2.38.0
[text/x-diff] v2-0007-meson-Add-Dpkglibdir-option.patch (2.8K, ../../[email protected]/8-v2-0007-meson-Add-Dpkglibdir-option.patch)
download | inline diff:
From d3dc1f361e73f7e2ee4732cc82eb5279596b548d Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 3 Nov 2023 10:06:00 -0700
Subject: [PATCH v2 7/7] meson: Add -Dpkglibdir option
Author:
Reviewed-by:
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
meson.build | 9 ++++++---
meson_options.txt | 3 +++
doc/src/sgml/installation.sgml | 18 +++++++++++++++---
3 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/meson.build b/meson.build
index 0f086757e17..9f38433285b 100644
--- a/meson.build
+++ b/meson.build
@@ -492,9 +492,12 @@ endif
dir_lib = get_option('libdir')
-dir_lib_pkg = dir_lib
-if not (dir_prefix_contains_pg or dir_lib_pkg.contains('pgsql') or dir_lib_pkg.contains('postgres'))
- dir_lib_pkg = dir_lib_pkg / pkg
+dir_lib_pkg = get_option('pkglibdir')
+if dir_lib_pkg == ''
+ dir_lib_pkg = dir_lib
+ if not (dir_prefix_contains_pg or dir_lib_pkg.contains('pgsql') or dir_lib_pkg.contains('postgres'))
+ dir_lib_pkg = dir_lib_pkg / pkg
+ endif
endif
dir_pgxs = dir_lib_pkg / 'pgxs'
diff --git a/meson_options.txt b/meson_options.txt
index be1b327f544..82441b4d524 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -70,6 +70,9 @@ option('darwin_sysroot', type: 'string', value: '',
option('rpath', type: 'boolean', value: true,
description: 'Embed shared library search path in executables')
+option('pkglibdir', type: 'string', value: '',
+ description: 'Directory to install / load dynamically loadable modules from')
+
# External dependencies
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index d7fefa0918c..8345bb68e92 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2254,9 +2254,21 @@ ninja install
<term><option>--libdir=<replaceable>DIRECTORY</replaceable></option></term>
<listitem>
<para>
- Sets the location to install libraries and dynamically loadable
- modules. The default is
- <filename><replaceable>PREFIX</replaceable>/lib</filename>.
+ Sets the location to install libraries. The default is
+ <filename><replaceable>PREFIX</replaceable>/lib</filename>. This
+ option, unless <xref linkend='configure-pkglibdir-meson'/> is
+ specified, also controls where dynamically loadable modules get
+ installed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="configure-pkglibdir-meson">
+ <term><option>-Dpkglibdir=<replaceable>DIRECTORY</replaceable></option></term>
+ <listitem>
+ <para>
+ Sets the location to dynamically loadable modules. The default is set
+ by <xref linkend='configure-libdir-meson'/>.
</para>
</listitem>
</varlistentry>
--
2.38.0
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
@ 2023-11-09 14:39 ` Peter Eisentraut <[email protected]>
1 sibling, 0 replies; 27+ messages in thread
From: Peter Eisentraut @ 2023-11-09 14:39 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 09.11.23 00:21, Andres Freund wrote:
> Example output:
This is very nice!
> $ ninja help
> [0/1 1 0%] Running external command help (wrapped by meson to set env)
> Code Targets:
> all Build everything other than documentation
> backend Build backend and related modules
> bin Build frontend binaries
> contrib Build contrib modules
> pl Build procedual languages
ok
> Documentation Targets:
> docs Build documentation in multi-page HTML format
> doc-html Build documentation in multi-page HTML format
> doc-man Build documentation in man page format
> doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with A4 pages
> doc/src/sgml/postgres-US.pdf Build documentation in PDF format, with US letter pages
> doc/src/sgml/postgres.html Build documentation in single-page HTML format
> alldocs Build documentation in all supported formats
>
> Installation Targets:
> install Install postgres, excluding documentation
This should probably read "Install everything other than documentation",
to mirror "all" above. (Otherwise one might think it installs just the
backend.)
> install-doc-html Install documentation in multi-page HTML format
> install-doc-man Install documentation in man page format
> install-docs Install documentation in multi-page HTML and man page formats
There is a mismatch between "docs" and "install-docs". (As was
previously discussed, I'm in the camp that "docs" should be html + man.)
> install-quiet Like "install", but installed files are not displayed
> install-world Install postgres, including multi-page HTML and man page documentation
Suggest "Install everything, including documentation" (matches "world").
> uninstall Remove installed files
>
> Other Targets:
> clean Remove all build products
> test Run all enabled tests (including contrib)
> world Build everything, including documentation
Shouldn't that be under "Code Targets"?
> help List important targets
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
@ 2023-11-14 20:16 ` Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
1 sibling, 1 reply; 27+ messages in thread
From: Peter Eisentraut @ 2023-11-14 20:16 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Some comments on your patches:
v2-0001-meson-Change-default-of-selinux-feature-option-to.patch
Ok
v2-0002-docs-Document-with-selinux-Dselinux-options-centr.patch
Ok, but "selinux" should be "SELinux" when referring to the product.
v2-0003-meson-docs-Add-doc-html-man-targets.patch
We have make targets "html" and "man", so I suggest we make the meson
targets the same.
v2-0004-meson-Add-world-target.patch
AFAICT, this world target doesn't include the man target. (Again, this
would all work better if we added "man" to "docs".)
v2-0005-docs-meson-Add-documentation-for-important-build-.patch
It's nice to document this, but it's weird that we only document the
meson targets, not the make targets.
v2-0006-meson-Add-help-target-build-docs-from-a-common-so.patch
Here also, making this consistent and uniform with make would be useful.
v2-0007-meson-Add-Dpkglibdir-option.patch
Normally, the pkgFOOdir variables are just FOOdir plus package name. I
don't feel comfortable allowing those to be separately set. We don't
allow that with configure; this just arose from a Debian patch.
The description "location to dynamically loadable modules" is too
narrow. Consider for example, another proposed patch, where we are
doing some preprocessing on postgres.bki at build time. Since that
makes postgres.bki platform dependent, it should really be moved from
share (datadir in configure parlance) to pkglibdir. So then we have
things in there that are not loadable modules. I don't know how that
affects Debian packaging, but this patch might not be the right one.
I suggest we leave this patch for a separate discussion.
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
@ 2023-11-15 00:22 ` Andres Freund <[email protected]>
2023-11-15 00:30 ` Re: meson documentation build open issues Andres Freund <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andres Freund @ 2023-11-15 00:22 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2023-11-14 21:16:13 +0100, Peter Eisentraut wrote:
> Some comments on your patches:
>
> v2-0001-meson-Change-default-of-selinux-feature-option-to.patch
>
> Ok
>
> v2-0002-docs-Document-with-selinux-Dselinux-options-centr.patch
>
> Ok, but "selinux" should be "SELinux" when referring to the product.
Will apply with that fix.
> v2-0003-meson-docs-Add-doc-html-man-targets.patch
>
> We have make targets "html" and "man", so I suggest we make the meson
> targets the same.
Hm, ok.
> v2-0004-meson-Add-world-target.patch
>
> AFAICT, this world target doesn't include the man target. (Again, this
> would all work better if we added "man" to "docs".)
I agree with that sentiment - I only moved to the current arrangement after
Tom argued forcefully against building both.
The situation in the make world is weird:
"make docs" in the toplevel builds both, because it's defined as
docs:
$(MAKE) -C doc all
Buf if you "make -C doc/src/sgml" (or are in doc/src/sgml), we only build
html, as the default target is explicitly just html:
# Make "html" the default target, since that is what most people tend
# to want to use.
html:
There's no real way of making the recursive-make and non-recursive ninja
coherent. There's no equivalent to default target in a sudirectory with ninja
(or non-recursive make).
> v2-0005-docs-meson-Add-documentation-for-important-build-.patch
>
> It's nice to document this, but it's weird that we only document the meson
> targets, not the make targets.
I think it'd have been good if we had documented the important targets with
make. But I don't think documenting them as a prerequisite to documenting the
meson targets makes much sense.
> v2-0006-meson-Add-help-target-build-docs-from-a-common-so.patch
>
> Here also, making this consistent and uniform with make would be useful.
What precisely are you referring to here? Also adding a help target? Or just
consistency between what the "docs" target does?
> v2-0007-meson-Add-Dpkglibdir-option.patch
>
> Normally, the pkgFOOdir variables are just FOOdir plus package name. I
> don't feel comfortable allowing those to be separately set. We don't allow
> that with configure; this just arose from a Debian patch.
Right - but Debian's desire seems quite sensible. The need to have multiple
postgres versions installed in parallel is quite widespread.
> The description "location to dynamically loadable modules" is too narrow.
> Consider for example, another proposed patch, where we are doing some
> preprocessing on postgres.bki at build time. Since that makes postgres.bki
> platform dependent, it should really be moved from share (datadir in
> configure parlance) to pkglibdir.
I think I cannot be faulted for documenting the current use of the directory
:).
Separately, I'm not really convinced that moving some build time values into
postgres.bki is useful, but that's a matter for a different thread.
> So then we have things in there that are not loadable modules. I don't know
> how that affects Debian packaging, but this patch might not be the right
> one.
I'm not really seeing why that'd affect pkglibdir being adjustable, besides
needing to tweak the description of pkglibdir?
> I suggest we leave this patch for a separate discussion.
Fair enough.
Thanks for the review,
Andres Freund
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
@ 2023-11-15 00:30 ` Andres Freund <[email protected]>
2023-11-17 18:53 ` Re: meson documentation build open issues Andres Freund <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andres Freund @ 2023-11-15 00:30 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2023-11-14 16:22:31 -0800, Andres Freund wrote:
> > v2-0004-meson-Add-world-target.patch
> >
> > AFAICT, this world target doesn't include the man target. (Again, this
> > would all work better if we added "man" to "docs".)
>
> I agree with that sentiment - I only moved to the current arrangement after
> Tom argued forcefully against building both.
Another message in this thread made me realize that I actually hadn't
implemented it at all - it was Tom in 969509c3f2e
In HEAD, also document how to build docs using Meson, and adjust
"ninja docs" to just build the HTML docs, for consistency with the
default behavior of doc/src/sgml/Makefile.
I think that change was just ill-advised, given that the top-level make target
actually *does* build both html and man:
> The situation in the make world is weird:
> "make docs" in the toplevel builds both, because it's defined as
>
> docs:
> $(MAKE) -C doc all
Notwithstanding this:
> Buf if you "make -C doc/src/sgml" (or are in doc/src/sgml), we only build
> html, as the default target is explicitly just html:
As the obvious thing for people that really just want to build html with ninja
would be to just use the doc-html (to-be-renamed to "html") target.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-15 00:30 ` Re: meson documentation build open issues Andres Freund <[email protected]>
@ 2023-11-17 18:53 ` Andres Freund <[email protected]>
2023-11-20 07:27 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andres Freund @ 2023-11-17 18:53 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2023-11-14 16:30:24 -0800, Andres Freund wrote:
> On 2023-11-14 16:22:31 -0800, Andres Freund wrote:
> > > v2-0004-meson-Add-world-target.patch
> > >
> > > AFAICT, this world target doesn't include the man target. (Again, this
> > > would all work better if we added "man" to "docs".)
> >
> > I agree with that sentiment - I only moved to the current arrangement after
> > Tom argued forcefully against building both.
>
> Another message in this thread made me realize that I actually hadn't
> implemented it at all - it was Tom in 969509c3f2e
>
> In HEAD, also document how to build docs using Meson, and adjust
> "ninja docs" to just build the HTML docs, for consistency with the
> default behavior of doc/src/sgml/Makefile.
>
>
> I think that change was just ill-advised, given that the top-level make target
> actually *does* build both html and man:
>
> > The situation in the make world is weird:
> > "make docs" in the toplevel builds both, because it's defined as
> >
> > docs:
> > $(MAKE) -C doc all
>
> Notwithstanding this:
>
> > Buf if you "make -C doc/src/sgml" (or are in doc/src/sgml), we only build
> > html, as the default target is explicitly just html:
>
> As the obvious thing for people that really just want to build html with ninja
> would be to just use the doc-html (to-be-renamed to "html") target.
I pushed the first two commits (the selinux stuff) and worked a bit more on
the subsequent ones.
- As requested, I've renamed the 'doc-html' and 'doc-man' targets to just 'html'
and 'man'. Which then seems to also necessitates renaming the existing
install-doc-{html,man}. I'm not so sure about this change, likely because I
use autocomplete to remember the spelling of ninja (or make) targets, which
is easier with [install-]doc-{html,man} than with [install-]{html,man}.
- I added a commit to change what 'docs' builds, undoing that part of
969509c3f2e. I also moved the 'all' target in doc/src/sgml/Makefile up to
the 'html' target to make things less confusing there, as discussed in the
thread referenced in the commit message.
Because of the 'html' target, Tom can still just build html easily.
- I renamed 'meson-targets.txt' to 'targets-meson.txt' and renamed other files
to match. One reason is that meson tries to prevent conflict between its
internal targets by prefixing them with 'meson-', and the old names
conflicted with that rule. If we ever wanted to add something similar for
make, the new naming also seems better.
- I added documentation for some developer targets (reformat-dat-files,
expand-dat-files, update-unicode)
I didn't move 'world' in the docs, as it doesn't quite seem right in the "code
targets" section?
I attached the pkglibdir thing again, even though I don't plan to push it or
really review it further. Thought it might still be interesting for Christoph.
Greetings,
Andres Freund
Attachments:
[text/x-diff] v3-0001-meson-docs-Add-html-man-targets-rename-install-do.patch (1.6K, ../../[email protected]/2-v3-0001-meson-docs-Add-html-man-targets-rename-install-do.patch)
download | inline diff:
From 52a94efbcac0520716c792a33f0216e22584c72a Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Wed, 15 Nov 2023 18:53:03 -0800
Subject: [PATCH v3 1/5] meson: docs: Add {html,man} targets, rename
install-doc-*
We have toplevel html, man targets in the autoconf build as well. It'd be odd
to have an 'html' target but have the install target be 'install-doc-html',
thus rename the install targets to match.
Reviewed-by: Christoph Berg <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/meson.build | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index 90e2c062fa8..e5a38e060c7 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -142,7 +142,8 @@ if docs_dep.found()
'--install-dir-contents', dir_doc_html, html],
build_always_stale: true, build_by_default: false,
)
- alias_target('install-doc-html', install_doc_html)
+ alias_target('html', html)
+ alias_target('install-html', install_doc_html)
# build and install multi-page html docs as part of docs target
docs += html
@@ -231,7 +232,8 @@ if docs_dep.found()
'--install-dirs', dir_man, '@INPUT@'],
build_always_stale: true, build_by_default: false,
)
- alias_target('install-doc-man', install_doc_man)
+ alias_target('man', man)
+ alias_target('install-man', install_doc_man)
# even though we don't want to build man pages as part of 'docs', we do want
# to install them as part of install-docs
--
2.38.0
[text/x-diff] v3-0002-docs-Change-what-docs-meson-target-builds.patch (2.6K, ../../[email protected]/3-v3-0002-docs-Change-what-docs-meson-target-builds.patch)
download | inline diff:
From 69b85a8224aaa52aa5896fbfa6952ae18c184219 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 17 Nov 2023 10:29:19 -0800
Subject: [PATCH v3 2/5] docs: Change what 'docs' meson target builds
This undoes the change in what the 'docs' target builds 969509c3f2e. Tom was
concerned with having a target to just build the html docs, which a prior
commit now provided explicitly.
A subsequent commit will overhaul the documentation for the documentation
targets.
While at it, move all target in doc/src/sgml/Makefile up to just after the
default "html" target, and add a comment explaining "all" is *not* the default
target.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
doc/src/sgml/Makefile | 4 ++--
doc/src/sgml/docguide.sgml | 2 +-
doc/src/sgml/meson.build | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/Makefile b/doc/src/sgml/Makefile
index 49d35dd0d6e..53100828a64 100644
--- a/doc/src/sgml/Makefile
+++ b/doc/src/sgml/Makefile
@@ -15,6 +15,8 @@
# Make "html" the default target, since that is what most people tend
# to want to use.
html:
+# Note that all is *not* the default target in this directory
+all: html man
# We don't need the tree-wide headers or install support here.
NO_GENERATED_HEADERS=yes
@@ -25,8 +27,6 @@ top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-all: html man
-
ifndef DBTOEPUB
DBTOEPUB = $(missing) dbtoepub
diff --git a/doc/src/sgml/docguide.sgml b/doc/src/sgml/docguide.sgml
index fd72b3975ad..b319621fdb8 100644
--- a/doc/src/sgml/docguide.sgml
+++ b/doc/src/sgml/docguide.sgml
@@ -442,7 +442,7 @@ LOGLEVEL=-Dorg.apache.commons.logging.simplelog.defaultlog=WARN
<para>
To build just the <acronym>HTML</acronym> version of the documentation:
<screen>
-<prompt>build$ </prompt><userinput>ninja docs</userinput>
+<prompt>build$ </prompt><userinput>ninja html</userinput>
</screen>
To build all forms of the documentation:
<screen>
diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index e5a38e060c7..dfdb1d0daa7 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -235,9 +235,9 @@ if docs_dep.found()
alias_target('man', man)
alias_target('install-man', install_doc_man)
- # even though we don't want to build man pages as part of 'docs', we do want
- # to install them as part of install-docs
+ # built and installed as part of the the docs target
installdocs += install_doc_man
+ docs += man
endif
--
2.38.0
[text/x-diff] v3-0003-meson-Add-world-target.patch (772B, ../../[email protected]/4-v3-0003-meson-Add-world-target.patch)
download | inline diff:
From 40f0eae1be25aec792638cfe48145c79de8bb8de Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 17 Nov 2023 10:29:32 -0800
Subject: [PATCH v3 3/5] meson: Add 'world' target
We have this for make as well.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
meson.build | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meson.build b/meson.build
index 286d7e42698..588788aad4d 100644
--- a/meson.build
+++ b/meson.build
@@ -3331,6 +3331,8 @@ alias_target('bin', bin_targets + [libpq_st])
alias_target('pl', pl_targets)
alias_target('contrib', contrib_targets)
alias_target('testprep', testprep_targets)
+
+alias_target('world', all_built, docs)
alias_target('install-world', install_quiet, installdocs)
--
2.38.0
[text/x-diff] v3-0004-meson-Document-build-targets-add-help-target.patch (9.5K, ../../[email protected]/5-v3-0004-meson-Document-build-targets-add-help-target.patch)
download | inline diff:
From 70c2253d1967ab4ffca9d0b241b111515e072118 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 3 Nov 2023 14:06:25 -0700
Subject: [PATCH v3 4/5] meson: Document build targets, add 'help' target
Author:
Reviewed-by:
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
meson.build | 7 +++
doc/src/sgml/Makefile | 5 +-
doc/src/sgml/docguide.sgml | 13 +++---
doc/src/sgml/filelist.sgml | 1 +
doc/src/sgml/generate-targets-meson.pl | 63 ++++++++++++++++++++++++++
doc/src/sgml/installation.sgml | 15 ++++++
doc/src/sgml/meson.build | 9 ++++
doc/src/sgml/targets-meson.txt | 43 ++++++++++++++++++
8 files changed, 148 insertions(+), 8 deletions(-)
create mode 100644 doc/src/sgml/generate-targets-meson.pl
create mode 100644 doc/src/sgml/targets-meson.txt
diff --git a/meson.build b/meson.build
index 588788aad4d..ee58ee7a065 100644
--- a/meson.build
+++ b/meson.build
@@ -3335,6 +3335,13 @@ alias_target('testprep', testprep_targets)
alias_target('world', all_built, docs)
alias_target('install-world', install_quiet, installdocs)
+run_target('help',
+ command: [
+ perl, '-ne', 'next if /^#/; print',
+ files('doc/src/sgml/targets-meson.txt'),
+ ]
+)
+
###############################################################
diff --git a/doc/src/sgml/Makefile b/doc/src/sgml/Makefile
index 53100828a64..2ef818900fd 100644
--- a/doc/src/sgml/Makefile
+++ b/doc/src/sgml/Makefile
@@ -55,7 +55,7 @@ override XSLTPROCFLAGS += --stringparam pg.version '$(VERSION)'
GENERATED_SGML = version.sgml \
features-supported.sgml features-unsupported.sgml errcodes-table.sgml \
- keywords-table.sgml wait_event_types.sgml
+ keywords-table.sgml targets-meson.sgml wait_event_types.sgml
ALLSGML := $(wildcard $(srcdir)/*.sgml $(srcdir)/ref/*.sgml) $(GENERATED_SGML)
@@ -110,6 +110,9 @@ keywords-table.sgml: $(top_srcdir)/src/include/parser/kwlist.h $(wildcard $(srcd
wait_event_types.sgml: $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt $(top_srcdir)/src/backend/utils/activity/generate-wait_event_types.pl
$(PERL) $(top_srcdir)/src/backend/utils/activity/generate-wait_event_types.pl --docs $<
+targets-meson.sgml: targets-meson.txt $(srcdir)/generate-targets-meson.pl
+ $(PERL) $(srcdir)/generate-targets-meson.pl $^ > $@
+
##
## Generation of some text files.
##
diff --git a/doc/src/sgml/docguide.sgml b/doc/src/sgml/docguide.sgml
index b319621fdb8..c129215dd3c 100644
--- a/doc/src/sgml/docguide.sgml
+++ b/doc/src/sgml/docguide.sgml
@@ -434,9 +434,9 @@ LOGLEVEL=-Dorg.apache.commons.logging.simplelog.defaultlog=WARN
<title>Building the Documentation with Meson</title>
<para>
- Two options are provided for building the documentation using Meson.
- Change to the <filename>build</filename> directory before running
- one of these commands, or add <option>-C build</option> to the command.
+ To build the documentation using Meson, change to the
+ <filename>build</filename> directory before running one of these commands,
+ or add <option>-C build</option> to the command.
</para>
<para>
@@ -444,10 +444,9 @@ LOGLEVEL=-Dorg.apache.commons.logging.simplelog.defaultlog=WARN
<screen>
<prompt>build$ </prompt><userinput>ninja html</userinput>
</screen>
- To build all forms of the documentation:
-<screen>
-<prompt>build$ </prompt><userinput>ninja alldocs</userinput>
-</screen>
+ For a list of other documentation targets see
+ <xref linkend="targets-meson-documentation"/>.
+
The output appears in the
subdirectory <filename>build/doc/src/sgml</filename>.
</para>
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index 4c63a7e7689..bd42b3ef16b 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -38,6 +38,7 @@
<!ENTITY high-availability SYSTEM "high-availability.sgml">
<!ENTITY installbin SYSTEM "install-binaries.sgml">
<!ENTITY installation SYSTEM "installation.sgml">
+<!ENTITY targets-meson SYSTEM "targets-meson.sgml">
<!ENTITY installw SYSTEM "install-windows.sgml">
<!ENTITY maintenance SYSTEM "maintenance.sgml">
<!ENTITY manage-ag SYSTEM "manage-ag.sgml">
diff --git a/doc/src/sgml/generate-targets-meson.pl b/doc/src/sgml/generate-targets-meson.pl
new file mode 100644
index 00000000000..56a94d6ead9
--- /dev/null
+++ b/doc/src/sgml/generate-targets-meson.pl
@@ -0,0 +1,63 @@
+#!/usr/bin/perl
+#
+# Generate the targets-meson.sgml file from targets-meson.txt
+# Copyright (c) 2000-2023, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+my $targets_meson_file = $ARGV[0];
+open my $targets_meson, '<', $targets_meson_file or die;
+
+print
+ "<!-- autogenerated from doc/src/sgml/targets-meson.txt, do not edit -->\n";
+
+# Find the start of each group of targets
+while (<$targets_meson>)
+{
+ next if /^#/;
+
+ if (/^(.*) Targets:$/)
+ {
+ my $targets = $1;
+ my $targets_id = lc $targets;
+
+ print qq(
+<sect3 id="targets-meson-$targets_id">
+ <title>$targets Targets</title>
+
+ <variablelist>
+);
+
+ # Each target in the group
+ while (<$targets_meson>)
+ {
+ next if /^#/;
+ last if !/^\s+([^ ]+)\s+(.+)/;
+
+ my $target = $1;
+ my $desc = $2;
+ my $target_id = $1;
+
+ $target_id =~ s/\//-/g;
+
+ print qq(
+ <varlistentry id="meson-target-${target_id}">
+ <term><option>${target}</option></term>
+ <listitem>
+ <para>
+ ${desc}
+ </para>
+ </listitem>
+ </varlistentry>
+);
+ }
+
+ print qq(
+ </variablelist>
+</sect3>
+);
+ }
+}
+
+close $targets_meson;
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 8e926a3a8cb..b23b35cd8e7 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -3200,6 +3200,21 @@ ninja install
</variablelist>
</sect3>
</sect2>
+
+ <sect2 id="targets-meson">
+ <title><literal>meson</literal> Build Targets</title>
+
+ <para>
+ Individual build targets can be built using <command>ninja</command> <replaceable>target</replaceable>.
+
+ When no target is specified, everything except documentation is
+ built. Individual build products can be built using the path/filename as
+ <replaceable>target</replaceable>.
+ </para>
+
+ &targets-meson;
+ </sect2>
+
</sect1>
<sect1 id="install-post">
diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index dfdb1d0daa7..e1a85dc607b 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -71,6 +71,15 @@ doc_generated += custom_target('keywords-table.sgml',
capture: true,
)
+doc_generated += custom_target('targets-meson.sgml',
+ input: files('targets-meson.txt'),
+ output: 'targets-meson.sgml',
+ command: [perl, files('generate-targets-meson.pl'), '@INPUT@'],
+ build_by_default: false,
+ install: false,
+ capture: true,
+)
+
# For everything else we need at least xmllint
if not xmllint_bin.found()
subdir_done()
diff --git a/doc/src/sgml/targets-meson.txt b/doc/src/sgml/targets-meson.txt
new file mode 100644
index 00000000000..2495790429e
--- /dev/null
+++ b/doc/src/sgml/targets-meson.txt
@@ -0,0 +1,43 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+#
+# Description of important meson targets, used for the 'help' target and
+# installation.sgml (via generate-targets-meson.pl). Right now the parsers are
+# extremely simple. Both parsers ignore comments. The help target prints
+# everything else. For xml everything without a leading newline is a group,
+# remaining lines are target separated by whitespace from their description
+#
+Code Targets:
+ all Build everything other than documentation
+ backend Build backend and related modules
+ bin Build frontend binaries
+ contrib Build contrib modules
+ pl Build procedual languages
+
+Developer Targets:
+ reformat-dat-files Rewrite catalog data files into standard format
+ expand-dat-files Expand all data files to include defaults
+ update-unicode Update unicode data to new version
+
+Documentation Targets:
+ html Build documentation in multi-page HTML format
+ man Build documentation in man page format
+ docs Build documentation in multi-page HTML and man page format
+ doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with A4 pages
+ doc/src/sgml/postgres-US.pdf Build documentation in PDF format, with US letter pages
+ doc/src/sgml/postgres.html Build documentation in single-page HTML format
+ alldocs Build documentation in all supported formats
+
+Installation Targets:
+ install Install postgres, excluding documentation
+ install-docs Install documentation in multi-page HTML and man page formats
+ install-html Install documentation in multi-page HTML format
+ install-man Install documentation in man page format
+ install-quiet Like "install", but installed files are not displayed
+ install-world Install postgres, including multi-page HTML and man page documentation
+ uninstall Remove installed files
+
+Other Targets:
+ clean Remove all build products
+ test Run all enabled tests (including contrib)
+ world Build everything, including documentation
+ help List important targets
--
2.38.0
[text/x-diff] v3-0005-meson-Add-Dpkglibdir-option.patch (2.8K, ../../[email protected]/6-v3-0005-meson-Add-Dpkglibdir-option.patch)
download | inline diff:
From 96d6424087cf2d35b81df032050e2c1fc07ee74a Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 3 Nov 2023 10:06:00 -0700
Subject: [PATCH v3 5/5] meson: Add -Dpkglibdir option
Author:
Reviewed-by:
Discussion: https://postgr.es/m/[email protected]
Backpatch:
---
meson.build | 9 ++++++---
meson_options.txt | 3 +++
doc/src/sgml/installation.sgml | 18 +++++++++++++++---
3 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/meson.build b/meson.build
index ee58ee7a065..ed4c664e6eb 100644
--- a/meson.build
+++ b/meson.build
@@ -492,9 +492,12 @@ endif
dir_lib = get_option('libdir')
-dir_lib_pkg = dir_lib
-if not (dir_prefix_contains_pg or dir_lib_pkg.contains('pgsql') or dir_lib_pkg.contains('postgres'))
- dir_lib_pkg = dir_lib_pkg / pkg
+dir_lib_pkg = get_option('pkglibdir')
+if dir_lib_pkg == ''
+ dir_lib_pkg = dir_lib
+ if not (dir_prefix_contains_pg or dir_lib_pkg.contains('pgsql') or dir_lib_pkg.contains('postgres'))
+ dir_lib_pkg = dir_lib_pkg / pkg
+ endif
endif
dir_pgxs = dir_lib_pkg / 'pgxs'
diff --git a/meson_options.txt b/meson_options.txt
index be1b327f544..82441b4d524 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -70,6 +70,9 @@ option('darwin_sysroot', type: 'string', value: '',
option('rpath', type: 'boolean', value: true,
description: 'Embed shared library search path in executables')
+option('pkglibdir', type: 'string', value: '',
+ description: 'Directory to install / load dynamically loadable modules from')
+
# External dependencies
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index b23b35cd8e7..3b6e49dbdbb 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2254,9 +2254,21 @@ ninja install
<term><option>--libdir=<replaceable>DIRECTORY</replaceable></option></term>
<listitem>
<para>
- Sets the location to install libraries and dynamically loadable
- modules. The default is
- <filename><replaceable>PREFIX</replaceable>/lib</filename>.
+ Sets the location to install libraries. The default is
+ <filename><replaceable>PREFIX</replaceable>/lib</filename>. This
+ option, unless <xref linkend='configure-pkglibdir-meson'/> is
+ specified, also controls where dynamically loadable modules get
+ installed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="configure-pkglibdir-meson">
+ <term><option>-Dpkglibdir=<replaceable>DIRECTORY</replaceable></option></term>
+ <listitem>
+ <para>
+ Sets the location to dynamically loadable modules. The default is set
+ by <xref linkend='configure-libdir-meson'/>.
</para>
</listitem>
</varlistentry>
--
2.38.0
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-15 00:30 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-17 18:53 ` Re: meson documentation build open issues Andres Freund <[email protected]>
@ 2023-11-20 07:27 ` Peter Eisentraut <[email protected]>
2023-11-21 01:56 ` Re: meson documentation build open issues Andres Freund <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Peter Eisentraut @ 2023-11-20 07:27 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 17.11.23 19:53, Andres Freund wrote:
> I pushed the first two commits (the selinux stuff) and worked a bit more on
> the subsequent ones.
Patches 0001 through 0004 look good to me.
Some possible small tweaks in 0004:
+ perl, '-ne', 'next if /^#/; print',
If you're going for super-brief mode, you could also use "perl -p" and
drop the "print".
Put at least two spaces between the "columns" in targets-meson.txt:
+ doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with
^^
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-15 00:30 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-17 18:53 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-20 07:27 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
@ 2023-11-21 01:56 ` Andres Freund <[email protected]>
2023-11-21 13:23 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andres Freund @ 2023-11-21 01:56 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2023-11-20 08:27:48 +0100, Peter Eisentraut wrote:
> On 17.11.23 19:53, Andres Freund wrote:
> > I pushed the first two commits (the selinux stuff) and worked a bit more on
> > the subsequent ones.
>
> Patches 0001 through 0004 look good to me.
Cool, I pushed them now.
> Some possible small tweaks in 0004:
>
> + perl, '-ne', 'next if /^#/; print',
>
> If you're going for super-brief mode, you could also use "perl -p" and drop
> the "print".
I thought this didn't add much, so I didn't go there.
> Put at least two spaces between the "columns" in targets-meson.txt:
>
> + doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with
> ^^
I did adopt this.
One remaining question is whether we should adjust install-doc-{html,man} to
be install-{html,man}, to match the docs targets.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-15 00:30 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-17 18:53 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-20 07:27 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-21 01:56 ` Re: meson documentation build open issues Andres Freund <[email protected]>
@ 2023-11-21 13:23 ` Peter Eisentraut <[email protected]>
2023-11-21 14:54 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Peter Eisentraut @ 2023-11-21 13:23 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 21.11.23 02:56, Andres Freund wrote:
> One remaining question is whether we should adjust install-doc-{html,man} to
> be install-{html,man}, to match the docs targets.
Ah didn't notice that one; yes please.
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: meson documentation build open issues
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-14 20:16 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-15 00:30 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-17 18:53 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-20 07:27 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
2023-11-21 01:56 ` Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-21 13:23 ` Re: meson documentation build open issues Peter Eisentraut <[email protected]>
@ 2023-11-21 14:54 ` Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Peter Eisentraut @ 2023-11-21 14:54 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 21.11.23 14:23, Peter Eisentraut wrote:
> On 21.11.23 02:56, Andres Freund wrote:
>> One remaining question is whether we should adjust
>> install-doc-{html,man} to
>> be install-{html,man}, to match the docs targets.
>
> Ah didn't notice that one; yes please.
I think this was done?
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0011-table_scan_bitmap_next_block-counts-lossy-and-exa.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 29 +++++++----------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..d2bf8f44d50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
* Create the TBMSharedIterator struct, with enough trailing space to
* serve the needs of the TBMIterateResult sub-struct.
*/
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0011-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 29 +++++++----------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..d2bf8f44d50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
* Create the TBMSharedIterator struct, with enough trailing space to
* serve the needs of the TBMIterateResult sub-struct.
*/
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 29 +++++++----------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..d2bf8f44d50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
* Create the TBMSharedIterator struct, with enough trailing space to
* serve the needs of the TBMIterateResult sub-struct.
*/
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0011-table_scan_bitmap_next_block-counts-lossy-and-exa.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 29 +++++++----------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..d2bf8f44d50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
* Create the TBMSharedIterator struct, with enough trailing space to
* serve the needs of the TBMIterateResult sub-struct.
*/
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 29 +++++++----------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..d2bf8f44d50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
* Create the TBMSharedIterator struct, with enough trailing space to
* serve the needs of the TBMIterateResult sub-struct.
*/
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0011-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v12 12/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..1dc4c99bf99 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v12-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 29 +++++++----------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..d2bf8f44d50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
* Create the TBMSharedIterator struct, with enough trailing space to
* serve the needs of the TBMIterateResult sub-struct.
*/
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0011-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0011-table_scan_bitmap_next_block-counts-lossy-and-exa.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size
@ 2024-02-16 01:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw)
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..1dc4c99bf9 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..432fae5296 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
end of thread, other threads:[~2024-02-16 01:13 UTC | newest]
Thread overview: 27+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-11-08 23:21 Re: meson documentation build open issues Andres Freund <[email protected]>
2023-11-09 14:39 ` Peter Eisentraut <[email protected]>
2023-11-14 20:16 ` Peter Eisentraut <[email protected]>
2023-11-15 00:22 ` Andres Freund <[email protected]>
2023-11-15 00:30 ` Andres Freund <[email protected]>
2023-11-17 18:53 ` Andres Freund <[email protected]>
2023-11-20 07:27 ` Peter Eisentraut <[email protected]>
2023-11-21 01:56 ` Andres Freund <[email protected]>
2023-11-21 13:23 ` Peter Eisentraut <[email protected]>
2023-11-21 14:54 ` Peter Eisentraut <[email protected]>
2024-02-16 01:13 [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v12 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
2024-02-16 01:13 [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox