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]> 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:08 Nathan Bossart <[email protected]> parent: 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:27 David Rowley <[email protected]> parent: 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:29 Nathan Bossart <[email protected]> parent: David Rowley <[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 22:08 Nathan Bossart <[email protected]> parent: 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