public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 01/10] Allow alternate compression methods for wal_compression
6+ messages / 3 participants
[nested] [flat]
* [PATCH 01/10] Allow alternate compression methods for wal_compression
@ 2021-02-27 04:03 Andrey Borodin <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Andrey Borodin @ 2021-02-27 04:03 UTC (permalink / raw)
TODO: bump XLOG_PAGE_MAGIC
---
doc/src/sgml/config.sgml | 17 +++++
src/backend/Makefile | 2 +-
src/backend/access/transam/xlog.c | 10 +++
src/backend/access/transam/xloginsert.c | 67 ++++++++++++++++---
src/backend/access/transam/xlogreader.c | 64 +++++++++++++++++-
src/backend/utils/misc/guc.c | 11 +++
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/access/xlog.h | 1 +
src/include/access/xlog_internal.h | 16 +++++
src/include/access/xlogrecord.h | 11 ++-
10 files changed, 187 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a218d78bef..7fb2a84626 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3072,6 +3072,23 @@ include_dir 'conf.d'
</listitem>
</varlistentry>
+ <varlistentry id="guc-wal-compression-method" xreflabel="wal_compression_method">
+ <term><varname>wal_compressionion_method</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>wal_compression_method</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ This parameter selects the compression method used to compress WAL when
+ <varname>wal_compression</varname> is enabled.
+ The supported methods are pglz and zlib.
+ The default value is <literal>pglz</literal>.
+ Only superusers can change this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-wal-init-zero" xreflabel="wal_init_zero">
<term><varname>wal_init_zero</varname> (<type>boolean</type>)
<indexterm>
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 0da848b1fd..3af216ddfc 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -48,7 +48,7 @@ OBJS = \
LIBS := $(filter-out -lpgport -lpgcommon, $(LIBS)) $(LDAP_LIBS_BE) $(ICU_LIBS)
# The backend doesn't need everything that's in LIBS, however
-LIBS := $(filter-out -lz -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
+LIBS := $(filter-out -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
ifeq ($(with_systemd),yes)
LIBS += -lsystemd
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f4d1ce5dea..15da91a8dd 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -99,6 +99,7 @@ bool EnableHotStandby = false;
bool fullPageWrites = true;
bool wal_log_hints = false;
bool wal_compression = false;
+int wal_compression_method = WAL_COMPRESSION_PGLZ;
char *wal_consistency_checking_string = NULL;
bool *wal_consistency_checking = NULL;
bool wal_init_zero = true;
@@ -180,6 +181,15 @@ const struct config_enum_entry recovery_target_action_options[] = {
{NULL, 0, false}
};
+/* Note that due to conditional compilation, offsets within the array are not static */
+const struct config_enum_entry wal_compression_options[] = {
+ {"pglz", WAL_COMPRESSION_PGLZ, false},
+#ifdef HAVE_LIBZ
+ {"zlib", WAL_COMPRESSION_ZLIB, false},
+#endif
+ {NULL, 0, false}
+};
+
/*
* Statistics for current checkpoint are collected in this global struct.
* Because only the checkpointer or a stand-alone backend can perform
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index 7052dc245e..a93b33464f 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -33,8 +33,18 @@
#include "storage/proc.h"
#include "utils/memutils.h"
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+/* zlib compressBound is not a macro */
+#define ZLIB_MAX_BLCKSZ BLCKSZ + (BLCKSZ>>12) + (BLCKSZ>>14) + (BLCKSZ>>25) + 13
+#else
+#define ZLIB_MAX_BLCKSZ 0
+#endif
+
/* Buffer size required to store a compressed version of backup block image */
-#define PGLZ_MAX_BLCKSZ PGLZ_MAX_OUTPUT(BLCKSZ)
+#define PGLZ_MAX_BLCKSZ PGLZ_MAX_OUTPUT(BLCKSZ)
+
+#define COMPRESS_BUFSIZE Max(PGLZ_MAX_BLCKSZ, ZLIB_MAX_BLCKSZ)
/*
* For each block reference registered with XLogRegisterBuffer, we fill in
@@ -58,7 +68,7 @@ typedef struct
* backup block data in XLogRecordAssemble() */
/* buffer to store a compressed version of backup block image */
- char compressed_page[PGLZ_MAX_BLCKSZ];
+ char compressed_page[COMPRESS_BUFSIZE];
} registered_buffer;
static registered_buffer *registered_buffers;
@@ -113,7 +123,8 @@ static XLogRecData *XLogRecordAssemble(RmgrId rmid, uint8 info,
XLogRecPtr RedoRecPtr, bool doPageWrites,
XLogRecPtr *fpw_lsn, int *num_fpi);
static bool XLogCompressBackupBlock(char *page, uint16 hole_offset,
- uint16 hole_length, char *dest, uint16 *dlen);
+ uint16 hole_length, char *dest,
+ uint16 *dlen, WalCompression compression);
/*
* Begin constructing a WAL record. This must be called before the
@@ -625,16 +636,26 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
cbimg.hole_length = 0;
}
+ bimg.bimg_info = (cbimg.hole_length == 0) ? 0 : BKPIMAGE_HAS_HOLE;
+
/*
* Try to compress a block image if wal_compression is enabled
*/
if (wal_compression)
{
+ int compression;
+ /* The current compression is stored in the WAL record */
+ wal_compression_name(wal_compression_method); /* Range check */
+ compression = walmethods[wal_compression_method].walmethod;
+ Assert(compression < (1 << BKPIMAGE_COMPRESS_BITS));
+ bimg.bimg_info |=
+ compression << BKPIMAGE_COMPRESS_OFFSET_BITS;
is_compressed =
XLogCompressBackupBlock(page, bimg.hole_offset,
cbimg.hole_length,
regbuf->compressed_page,
- &compressed_len);
+ &compressed_len,
+ wal_compression_method);
}
/*
@@ -652,8 +673,6 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
rdt_datas_last->next = ®buf->bkp_rdatas[0];
rdt_datas_last = rdt_datas_last->next;
- bimg.bimg_info = (cbimg.hole_length == 0) ? 0 : BKPIMAGE_HAS_HOLE;
-
/*
* If WAL consistency checking is enabled for the resource manager
* of this WAL record, a full-page image is included in the record
@@ -827,7 +846,7 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
*/
static bool
XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
- char *dest, uint16 *dlen)
+ char *dest, uint16 *dlen, WalCompression compression)
{
int32 orig_len = BLCKSZ - hole_length;
int32 len;
@@ -853,12 +872,42 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
else
source = page;
+ switch (compression)
+ {
+ case WAL_COMPRESSION_PGLZ:
+ len = pglz_compress(source, orig_len, dest, PGLZ_strategy_default);
+ break;
+
+#ifdef HAVE_LIBZ
+ case WAL_COMPRESSION_ZLIB:
+ {
+ unsigned long len_l = COMPRESS_BUFSIZE;
+ int ret;
+ ret = compress2((Bytef*)dest, &len_l, (Bytef*)source, orig_len, 1);
+ if (ret != Z_OK)
+ len_l = -1;
+ len = len_l;
+ break;
+ }
+#endif
+
+ default:
+ /*
+ * It should be impossible to get here for unsupported algorithms,
+ * which cannot be assigned if they're not enabled at compile time.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("unknown compression method requested: %d(%s)",
+ compression, wal_compression_name(compression))));
+
+ }
+
/*
- * We recheck the actual size even if pglz_compress() reports success and
+ * We recheck the actual size even if compression reports success and
* see if the number of bytes saved by compression is larger than the
* length of extra data needed for the compressed version of block image.
*/
- len = pglz_compress(source, orig_len, dest, PGLZ_strategy_default);
if (len >= 0 &&
len + extra_bytes < orig_len)
{
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 42738eb940..0d8830fc50 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -33,6 +33,10 @@
#include "utils/memutils.h"
#endif
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
static void report_invalid_record(XLogReaderState *state, const char *fmt,...)
pg_attribute_printf(2, 3);
static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength);
@@ -1535,6 +1539,30 @@ XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *len)
}
}
+/* This is a mapping indexed by wal_compression */
+// XXX: maybe this is better done as a GUC hook to assign the 1) method; and 2) level
+struct walcompression walmethods[] = {
+ {"pglz", WAL_COMPRESSION_PGLZ},
+ {"zlib", WAL_COMPRESSION_ZLIB},
+};
+
+/*
+ * Return a statically allocated string associated with the given compression
+ * method.
+ * This is here to be visible to frontend tools like pg_rewind.
+ */
+const char *
+wal_compression_name(WalCompression compression)
+{
+ /*
+ * This could index into the guc array, except that it's compiled
+ * conditionally and unsupported methods are elided.
+ */
+ if (compression < sizeof(walmethods)/sizeof(*walmethods))
+ return walmethods[compression].name;
+ return "???";
+}
+
/*
* Restore a full-page image from a backup block attached to an XLOG record.
*
@@ -1557,9 +1585,41 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
if (bkpb->bimg_info & BKPIMAGE_IS_COMPRESSED)
{
+ int compression_method = BKPIMAGE_COMPRESSION(bkpb->bimg_info);
/* If a backup block image is compressed, decompress it */
- if (pglz_decompress(ptr, bkpb->bimg_len, tmp.data,
- BLCKSZ - bkpb->hole_length, true) < 0)
+ int32 decomp_result = -1;
+ switch (compression_method)
+ {
+ case WAL_COMPRESSION_PGLZ:
+ decomp_result = pglz_decompress(ptr, bkpb->bimg_len, tmp.data,
+ BLCKSZ - bkpb->hole_length, true);
+ break;
+
+#ifdef HAVE_LIBZ
+ case WAL_COMPRESSION_ZLIB:
+ {
+ unsigned long decomp_result_l;
+ decomp_result_l = BLCKSZ - bkpb->hole_length;
+ if (uncompress((Bytef*)tmp.data, &decomp_result_l,
+ (Bytef*)ptr, bkpb->bimg_len) == Z_OK)
+ decomp_result = decomp_result_l;
+ else
+ decomp_result = -1;
+ break;
+ }
+#endif
+
+ default:
+ report_invalid_record(record, "image at %X/%X is compressed with unsupported codec, block %d (%d/%s)",
+ (uint32) (record->ReadRecPtr >> 32),
+ (uint32) record->ReadRecPtr,
+ block_id,
+ compression_method,
+ wal_compression_name(compression_method));
+ return false;
+ }
+
+ if (decomp_result < 0)
{
report_invalid_record(record, "invalid compressed image at %X/%X, block %d",
LSN_FORMAT_ARGS(record->ReadRecPtr),
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 855076b1fd..8084027465 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -508,6 +508,7 @@ extern const struct config_enum_entry archive_mode_options[];
extern const struct config_enum_entry recovery_target_action_options[];
extern const struct config_enum_entry sync_method_options[];
extern const struct config_enum_entry dynamic_shared_memory_options[];
+extern const struct config_enum_entry wal_compression_options[];
/*
* GUC option variables that are exported from this module
@@ -4721,6 +4722,16 @@ static struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
+ {
+ {"wal_compression_method", PGC_SIGHUP, WAL_SETTINGS,
+ gettext_noop("Set the method used to compress full page images in the WAL."),
+ NULL
+ },
+ &wal_compression_method,
+ WAL_COMPRESSION_PGLZ, wal_compression_options,
+ NULL, NULL, NULL
+ },
+
{
{"dynamic_shared_memory_type", PGC_POSTMASTER, RESOURCES_MEM,
gettext_noop("Selects the dynamic shared memory implementation used."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index f46c2dd7a8..ef69a94492 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -213,6 +213,7 @@
# open_sync
#full_page_writes = on # recover from partial page writes
#wal_compression = off # enable compression of full-page writes
+#wal_compression_method = pglz # pglz, zlib
#wal_log_hints = off # also do full page writes of non-critical updates
# (change requires restart)
#wal_init_zero = on # zero-fill new WAL files
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 6d384d3ce6..fa2e5c611f 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -117,6 +117,7 @@ extern bool EnableHotStandby;
extern bool fullPageWrites;
extern bool wal_log_hints;
extern bool wal_compression;
+extern int wal_compression_method;
extern bool wal_init_zero;
extern bool wal_recycle;
extern bool *wal_consistency_checking;
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index b23e286406..b000a21557 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -324,4 +324,20 @@ extern bool InArchiveRecovery;
extern bool StandbyMode;
extern char *recoveryRestoreCommand;
+struct walcompression
+{
+ char *name;
+ int walmethod; /* Compression method to be stored in WAL */
+};
+
+extern struct walcompression walmethods[];
+
+typedef enum WalCompression
+{
+ WAL_COMPRESSION_PGLZ,
+ WAL_COMPRESSION_ZLIB,
+} WalCompression;
+
+extern const char *wal_compression_name(WalCompression compression);
+
#endif /* XLOG_INTERNAL_H */
diff --git a/src/include/access/xlogrecord.h b/src/include/access/xlogrecord.h
index 80c92a2498..7107cf6186 100644
--- a/src/include/access/xlogrecord.h
+++ b/src/include/access/xlogrecord.h
@@ -114,7 +114,7 @@ typedef struct XLogRecordBlockHeader
* present is (BLCKSZ - <length of "hole" bytes>).
*
* Additionally, when wal_compression is enabled, we will try to compress full
- * page images using the PGLZ compression algorithm, after removing the "hole".
+ * page images, after removing the "hole".
* This can reduce the WAL volume, but at some extra cost of CPU spent
* on the compression during WAL logging. In this case, since the "hole"
* length cannot be calculated by subtracting the number of page image bytes
@@ -147,6 +147,15 @@ typedef struct XLogRecordBlockImageHeader
#define BKPIMAGE_IS_COMPRESSED 0x02 /* page image is compressed */
#define BKPIMAGE_APPLY 0x04 /* page image should be restored during
* replay */
+#define BKPIMAGE_COMPRESS_METHOD1 0x08 /* bits to encode compression method */
+#define BKPIMAGE_COMPRESS_METHOD2 0x10 /* 0=pglz; 1=zlib; */
+
+/* How many bits to shift to extract compression */
+#define BKPIMAGE_COMPRESS_OFFSET_BITS 3
+/* How many bits are for compression */
+#define BKPIMAGE_COMPRESS_BITS 2
+/* Extract the compression from the bimg_info */
+#define BKPIMAGE_COMPRESSION(info) ((info >> BKPIMAGE_COMPRESS_OFFSET_BITS) & ((1<<BKPIMAGE_COMPRESS_BITS) - 1))
/*
* Extra header information used when page image has "hole" and
--
2.17.0
--jozmn01XJZjDjM3N
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0002-Run-011_crash_recovery.pl-with-wal_level-minimal.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-18 21:02 David Rowley <[email protected]>
2024-03-18 21:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: David Rowley @ 2024-03-18 21:02 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On Tue, 19 Mar 2024 at 06:30, Nathan Bossart <[email protected]> wrote:
> Here is a more fleshed-out version of what I believe David is proposing.
> On my machine, the gains aren't quite as impressive (~8.8s to ~5.2s for the
> test_popcount benchmark). I assume this is because this patch turns
> pg_popcount() into a function pointer, which is what the AVX512 patches do,
> too. I left out the 32-bit section from pg_popcount_fast(), but I'll admit
> that I'm not yet 100% sure that we can assume we're on a 64-bit system
> there.
I looked at your latest patch and tried out the performance on a Zen4
running windows and a Zen2 running on Linux. As follows:
AMD 3990x:
master:
postgres=# select drive_popcount(10000000, 1024);
Time: 11904.078 ms (00:11.904)
Time: 11907.176 ms (00:11.907)
Time: 11927.983 ms (00:11.928)
patched:
postgres=# select drive_popcount(10000000, 1024);
Time: 3641.271 ms (00:03.641)
Time: 3610.934 ms (00:03.611)
Time: 3663.423 ms (00:03.663)
AMD 7945HX Windows
master:
postgres=# select drive_popcount(10000000, 1024);
Time: 9832.845 ms (00:09.833)
Time: 9844.460 ms (00:09.844)
Time: 9858.608 ms (00:09.859)
patched:
postgres=# select drive_popcount(10000000, 1024);
Time: 3427.942 ms (00:03.428)
Time: 3364.262 ms (00:03.364)
Time: 3413.407 ms (00:03.413)
The only thing I'd question in the patch is in pg_popcount_fast(). It
looks like you've opted to not do the 32-bit processing on 32-bit
machines. I think that's likely still worth coding in a similar way to
how pg_popcount_slow() works. i.e. use "#if SIZEOF_VOID_P >= 8".
Probably one day we'll remove that code, but it seems strange to have
pg_popcount_slow() do it and not pg_popcount_fast().
> IMHO this work is arguably a prerequisite for the AVX512 work, as turning
> pg_popcount() into a function pointer will likely regress performance for
> folks on systems without AVX512 otherwise.
I think so too.
David
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Popcount optimization using AVX512
2024-03-18 21:02 Re: Popcount optimization using AVX512 David Rowley <[email protected]>
@ 2024-03-18 21:08 ` Nathan Bossart <[email protected]>
2024-03-18 21:27 ` Re: Popcount optimization using AVX512 David Rowley <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Bossart @ 2024-03-18 21:08 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On Tue, Mar 19, 2024 at 10:02:18AM +1300, David Rowley wrote:
> I looked at your latest patch and tried out the performance on a Zen4
> running windows and a Zen2 running on Linux. As follows:
Thanks for taking a look.
> The only thing I'd question in the patch is in pg_popcount_fast(). It
> looks like you've opted to not do the 32-bit processing on 32-bit
> machines. I think that's likely still worth coding in a similar way to
> how pg_popcount_slow() works. i.e. use "#if SIZEOF_VOID_P >= 8".
> Probably one day we'll remove that code, but it seems strange to have
> pg_popcount_slow() do it and not pg_popcount_fast().
The only reason I left it out was because I couldn't convince myself that
it wasn't dead code, given we assume that popcntq is available in
pg_popcount64_fast() today. But I don't see any harm in adding that just
in case.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Popcount optimization using AVX512
2024-03-18 21:02 Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 21:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
@ 2024-03-18 21:27 ` David Rowley <[email protected]>
2024-03-18 21:29 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: David Rowley @ 2024-03-18 21:27 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On Tue, 19 Mar 2024 at 10:08, Nathan Bossart <[email protected]> wrote:
>
> On Tue, Mar 19, 2024 at 10:02:18AM +1300, David Rowley wrote:
> > The only thing I'd question in the patch is in pg_popcount_fast(). It
> > looks like you've opted to not do the 32-bit processing on 32-bit
> > machines. I think that's likely still worth coding in a similar way to
> > how pg_popcount_slow() works. i.e. use "#if SIZEOF_VOID_P >= 8".
> > Probably one day we'll remove that code, but it seems strange to have
> > pg_popcount_slow() do it and not pg_popcount_fast().
>
> The only reason I left it out was because I couldn't convince myself that
> it wasn't dead code, given we assume that popcntq is available in
> pg_popcount64_fast() today. But I don't see any harm in adding that just
> in case.
It's probably more of a case of using native instructions rather than
ones that might be implemented only via microcode. For the record, I
don't know if that would be the case for popcntq on x86 32-bit and I
don't have the hardware to test it. It just seems less risky just to
do it.
David
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Popcount optimization using AVX512
2024-03-18 21:02 Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 21:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-18 21:27 ` Re: Popcount optimization using AVX512 David Rowley <[email protected]>
@ 2024-03-18 21:29 ` Nathan Bossart <[email protected]>
2024-03-18 22:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Bossart @ 2024-03-18 21:29 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On Tue, Mar 19, 2024 at 10:27:58AM +1300, David Rowley wrote:
> On Tue, 19 Mar 2024 at 10:08, Nathan Bossart <[email protected]> wrote:
>> On Tue, Mar 19, 2024 at 10:02:18AM +1300, David Rowley wrote:
>> > The only thing I'd question in the patch is in pg_popcount_fast(). It
>> > looks like you've opted to not do the 32-bit processing on 32-bit
>> > machines. I think that's likely still worth coding in a similar way to
>> > how pg_popcount_slow() works. i.e. use "#if SIZEOF_VOID_P >= 8".
>> > Probably one day we'll remove that code, but it seems strange to have
>> > pg_popcount_slow() do it and not pg_popcount_fast().
>>
>> The only reason I left it out was because I couldn't convince myself that
>> it wasn't dead code, given we assume that popcntq is available in
>> pg_popcount64_fast() today. But I don't see any harm in adding that just
>> in case.
>
> It's probably more of a case of using native instructions rather than
> ones that might be implemented only via microcode. For the record, I
> don't know if that would be the case for popcntq on x86 32-bit and I
> don't have the hardware to test it. It just seems less risky just to
> do it.
Agreed. Will send an updated patch shortly.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Popcount optimization using AVX512
2024-03-18 21:02 Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 21:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-18 21:27 ` Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 21:29 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
@ 2024-03-18 22:08 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Nathan Bossart @ 2024-03-18 22:08 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On Mon, Mar 18, 2024 at 04:29:19PM -0500, Nathan Bossart wrote:
> Agreed. Will send an updated patch shortly.
As promised...
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v4-0001-inline-function-calls-in-pg_popcount-when-possibl.patch (7.5K, ../../20240318220845.GA776042@nathanxps13/2-v4-0001-inline-function-calls-in-pg_popcount-when-possibl.patch)
download | inline diff:
From b673663b1d1344549cbd0912220f96ba1712afc6 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 18 Mar 2024 12:18:15 -0500
Subject: [PATCH v4 1/1] inline function calls in pg_popcount() when possible
---
src/include/port/pg_bitutils.h | 5 +-
src/port/pg_bitutils.c | 155 +++++++++++++++++++++++++--------
2 files changed, 121 insertions(+), 39 deletions(-)
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 46bf4f0103..53e5239717 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -302,17 +302,16 @@ pg_ceil_log2_64(uint64 num)
/* Attempt to use the POPCNT instruction, but perform a runtime check first */
extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
+extern PGDLLIMPORT uint64 (*pg_popcount) (const char *buf, int bytes);
#else
/* Use a portable implementation -- no need for a function pointer. */
extern int pg_popcount32(uint32 word);
extern int pg_popcount64(uint64 word);
+extern uint64 pg_popcount(const char *buf, int bytes);
#endif /* TRY_POPCNT_FAST */
-/* Count the number of one-bits in a byte array */
-extern uint64 pg_popcount(const char *buf, int bytes);
-
/*
* Rotate the bits of "word" to the right/left by n bits.
*/
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 640a89561a..1197696e97 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -103,18 +103,22 @@ const uint8 pg_number_of_ones[256] = {
4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
};
-static int pg_popcount32_slow(uint32 word);
-static int pg_popcount64_slow(uint64 word);
+static inline int pg_popcount32_slow(uint32 word);
+static inline int pg_popcount64_slow(uint64 word);
+static uint64 pg_popcount_slow(const char *buf, int bytes);
#ifdef TRY_POPCNT_FAST
static bool pg_popcount_available(void);
static int pg_popcount32_choose(uint32 word);
static int pg_popcount64_choose(uint64 word);
-static int pg_popcount32_fast(uint32 word);
-static int pg_popcount64_fast(uint64 word);
+static uint64 pg_popcount_choose(const char *buf, int bytes);
+static inline int pg_popcount32_fast(uint32 word);
+static inline int pg_popcount64_fast(uint64 word);
+static uint64 pg_popcount_fast(const char *buf, int bytes);
int (*pg_popcount32) (uint32 word) = pg_popcount32_choose;
int (*pg_popcount64) (uint64 word) = pg_popcount64_choose;
+uint64 (*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose;
#endif /* TRY_POPCNT_FAST */
#ifdef TRY_POPCNT_FAST
@@ -151,11 +155,13 @@ pg_popcount32_choose(uint32 word)
{
pg_popcount32 = pg_popcount32_fast;
pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
}
else
{
pg_popcount32 = pg_popcount32_slow;
pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
}
return pg_popcount32(word);
@@ -168,21 +174,42 @@ pg_popcount64_choose(uint64 word)
{
pg_popcount32 = pg_popcount32_fast;
pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
}
else
{
pg_popcount32 = pg_popcount32_slow;
pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
}
return pg_popcount64(word);
}
+static uint64
+pg_popcount_choose(const char *buf, int bytes)
+{
+ if (pg_popcount_available())
+ {
+ pg_popcount32 = pg_popcount32_fast;
+ pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
+ }
+ else
+ {
+ pg_popcount32 = pg_popcount32_slow;
+ pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
+ }
+
+ return pg_popcount(buf, bytes);
+}
+
/*
* pg_popcount32_fast
* Return the number of 1 bits set in word
*/
-static int
+static inline int
pg_popcount32_fast(uint32 word)
{
#ifdef _MSC_VER
@@ -199,7 +226,7 @@ __asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
* pg_popcount64_fast
* Return the number of 1 bits set in word
*/
-static int
+static inline int
pg_popcount64_fast(uint64 word)
{
#ifdef _MSC_VER
@@ -212,6 +239,52 @@ __asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc");
#endif
}
+/*
+ * pg_popcount_fast
+ * Returns the number of 1-bits in buf
+ */
+static uint64
+pg_popcount_fast(const char *buf, int bytes)
+{
+ uint64 popcnt = 0;
+
+#if SIZEOF_VOID_P >= 8
+ /* Process in 64-bit chunks if the buffer is aligned. */
+ if (buf == (const char *) TYPEALIGN(8, buf))
+ {
+ const uint64 *words = (const uint64 *) buf;
+
+ while (bytes >= 8)
+ {
+ popcnt += pg_popcount64_fast(*words++);
+ bytes -= 8;
+ }
+
+ buf = (const char *) words;
+ }
+#else
+ /* Process in 32-bit chunks if the buffer is aligned. */
+ if (buf == (const char *) TYPEALIGN(4, buf))
+ {
+ const uint32 *words = (const uint32 *) buf;
+
+ while (bytes >= 4)
+ {
+ popcnt += pg_popcount32_fast(*words++);
+ bytes -= 4;
+ }
+
+ buf = (const char *) words;
+ }
+#endif
+
+ /* Process any remaining bytes */
+ while (bytes--)
+ popcnt += pg_number_of_ones[(unsigned char) *buf++];
+
+ return popcnt;
+}
+
#endif /* TRY_POPCNT_FAST */
@@ -219,7 +292,7 @@ __asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc");
* pg_popcount32_slow
* Return the number of 1 bits set in word
*/
-static int
+static inline int
pg_popcount32_slow(uint32 word)
{
#ifdef HAVE__BUILTIN_POPCOUNT
@@ -241,7 +314,7 @@ pg_popcount32_slow(uint32 word)
* pg_popcount64_slow
* Return the number of 1 bits set in word
*/
-static int
+static inline int
pg_popcount64_slow(uint64 word)
{
#ifdef HAVE__BUILTIN_POPCOUNT
@@ -265,35 +338,12 @@ pg_popcount64_slow(uint64 word)
#endif /* HAVE__BUILTIN_POPCOUNT */
}
-#ifndef TRY_POPCNT_FAST
-
/*
- * When the POPCNT instruction is not available, there's no point in using
- * function pointers to vary the implementation between the fast and slow
- * method. We instead just make these actual external functions when
- * TRY_POPCNT_FAST is not defined. The compiler should be able to inline
- * the slow versions here.
- */
-int
-pg_popcount32(uint32 word)
-{
- return pg_popcount32_slow(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
- return pg_popcount64_slow(word);
-}
-
-#endif /* !TRY_POPCNT_FAST */
-
-/*
- * pg_popcount
+ * pg_popcount_slow
* Returns the number of 1-bits in buf
*/
-uint64
-pg_popcount(const char *buf, int bytes)
+static uint64
+pg_popcount_slow(const char *buf, int bytes)
{
uint64 popcnt = 0;
@@ -305,7 +355,7 @@ pg_popcount(const char *buf, int bytes)
while (bytes >= 8)
{
- popcnt += pg_popcount64(*words++);
+ popcnt += pg_popcount64_slow(*words++);
bytes -= 8;
}
@@ -319,7 +369,7 @@ pg_popcount(const char *buf, int bytes)
while (bytes >= 4)
{
- popcnt += pg_popcount32(*words++);
+ popcnt += pg_popcount32_slow(*words++);
bytes -= 4;
}
@@ -333,3 +383,36 @@ pg_popcount(const char *buf, int bytes)
return popcnt;
}
+
+#ifndef TRY_POPCNT_FAST
+
+/*
+ * When the POPCNT instruction is not available, there's no point in using
+ * function pointers to vary the implementation between the fast and slow
+ * method. We instead just make these actual external functions when
+ * TRY_POPCNT_FAST is not defined. The compiler should be able to inline
+ * the slow versions here.
+ */
+int
+pg_popcount32(uint32 word)
+{
+ return pg_popcount32_slow(word);
+}
+
+int
+pg_popcount64(uint64 word)
+{
+ return pg_popcount64_slow(word);
+}
+
+/*
+ * pg_popcount
+ * Returns the number of 1-bits in buf
+ */
+uint64
+pg_popcount(const char *buf, int bytes)
+{
+ return pg_popcount_slow(buf, bytes);
+}
+
+#endif /* !TRY_POPCNT_FAST */
--
2.25.1
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-03-18 22:08 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-27 04:03 [PATCH 01/10] Allow alternate compression methods for wal_compression Andrey Borodin <[email protected]>
2024-03-18 21:02 Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 21:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-18 21:27 ` Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 21:29 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-18 22:08 ` Re: Popcount optimization using AVX512 Nathan Bossart <[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