So I tried running vacuum full in pgbench of your 10-column table, max_wal_size=32GB. I didn't move pgdata to an in-memory pgdata, but this is on NVMe so pretty fast anyway.
pgbench -c1 -t30 -n -f vacuumfull.sql. Current master: latency average = 2885.550 ms latency stddev = 1771.170 ms tps = 0.346554 (without initial connection time) With the recompression code ifdef'ed out (pretty much like in your patch): latency average = 2481.336 ms latency stddev = 1011.738 ms tps = 0.403008 (without initial connection time) With toast_get_compression_id as a static inline, like in the attach patch: latency average = 2520.982 ms latency stddev = 1043.042 ms tps = 0.396671 (without initial connection time) It seems to me that most of the overhead is the function call for toast_get_compression_id(), so we should get rid of that. Now, while this patch does seem to work correctly, it raises a number of weird cpluspluscheck warnings, which I think are attributable to the new macro definitions. I didn't look into it closely, but I suppose it should be fixable given sufficient effort: In file included from /tmp/cpluspluscheck.yuQqS5/test.cpp:2: /pgsql/source/master//src/include/access/toast_compression.h: In function ‘ToastCompressionId toast_get_compression_id(varlena*)’: /pgsql/source/master//src/include/postgres.h:392:46: warning: comparison of integer expressions of different signedness: ‘uint32’ {aka ‘unsigned int’} and ‘int32’ {aka ‘int’} [-Wsign-compare] (VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer) < \ /pgsql/source/master//src/include/access/toast_compression.h:109:7: note: in expansion of macro ‘VARATT_EXTERNAL_IS_COMPRESSED’ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master//src/include/postgres.h:374:30: error: invalid conversion from ‘uint32’ {aka ‘unsigned int’} to ‘ToastCompressionId’ [-fpermissive] ((toast_pointer).va_extinfo >> VARLENA_EXTSIZE_BITS) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master//src/include/access/toast_compression.h:110:11: note: in expansion of macro ‘VARATT_EXTERNAL_GET_COMPRESS_METHOD’ cmid = VARATT_EXTERNAL_GET_COMPRESS_METHOD(toast_pointer); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master//src/include/postgres.h:368:53: error: invalid conversion from ‘uint32’ {aka ‘unsigned int’} to ‘ToastCompressionId’ [-fpermissive] (((varattrib_4b *) (PTR))->va_compressed.va_tcinfo >> VARLENA_EXTSIZE_BITS) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master//src/include/access/toast_compression.h:113:10: note: in expansion of macro ‘VARDATA_COMPRESSED_GET_COMPRESS_METHOD’ cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /tmp/cpluspluscheck.yuQqS5/test.cpp:2: /pgsql/source/master/src/include/access/toast_compression.h: In function ‘ToastCompressionId toast_get_compression_id(varlena*)’: /pgsql/source/master//src/include/postgres.h:392:46: warning: comparison of integer expressions of different signedness: ‘uint32’ {aka ‘unsigned int’} and ‘int32’ {aka ‘int’} [-Wsign-compare] (VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer) < \ /pgsql/source/master/src/include/access/toast_compression.h:109:7: note: in expansion of macro ‘VARATT_EXTERNAL_IS_COMPRESSED’ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master//src/include/postgres.h:374:30: error: invalid conversion from ‘uint32’ {aka ‘unsigned int’} to ‘ToastCompressionId’ [-fpermissive] ((toast_pointer).va_extinfo >> VARLENA_EXTSIZE_BITS) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master/src/include/access/toast_compression.h:110:11: note: in expansion of macro ‘VARATT_EXTERNAL_GET_COMPRESS_METHOD’ cmid = VARATT_EXTERNAL_GET_COMPRESS_METHOD(toast_pointer); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master//src/include/postgres.h:368:53: error: invalid conversion from ‘uint32’ {aka ‘unsigned int’} to ‘ToastCompressionId’ [-fpermissive] (((varattrib_4b *) (PTR))->va_compressed.va_tcinfo >> VARLENA_EXTSIZE_BITS) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ /pgsql/source/master/src/include/access/toast_compression.h:113:10: note: in expansion of macro ‘VARDATA_COMPRESSED_GET_COMPRESS_METHOD’ cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- Álvaro Herrera Valdivia, Chile
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index 9e9d4457ac..38688e7cfc 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -247,36 +247,6 @@ lz4_decompress_datum_slice(const struct varlena *value, int32 slicelength) #endif } -/* - * Extract compression ID from a varlena. - * - * Returns TOAST_INVALID_COMPRESSION_ID if the varlena is not compressed. - */ -ToastCompressionId -toast_get_compression_id(struct varlena *attr) -{ - ToastCompressionId cmid = TOAST_INVALID_COMPRESSION_ID; - - /* - * If it is stored externally then fetch the compression method id from - * the external toast pointer. If compressed inline, fetch it from the - * toast compression header. - */ - if (VARATT_IS_EXTERNAL_ONDISK(attr)) - { - struct varatt_external toast_pointer; - - VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); - - if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) - cmid = VARATT_EXTERNAL_GET_COMPRESS_METHOD(toast_pointer); - } - else if (VARATT_IS_COMPRESSED(attr)) - cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); - - return cmid; -} - /* * CompressionNameToMethod - Get compression method from compression name * diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h index 9e2c1cbe1a..8756de77d8 100644 --- a/src/include/access/toast_compression.h +++ b/src/include/access/toast_compression.h @@ -13,6 +13,8 @@ #ifndef TOAST_COMPRESSION_H #define TOAST_COMPRESSION_H +#include "access/detoast.h" + /* * GUC support. * @@ -78,8 +80,41 @@ extern struct varlena *lz4_decompress_datum_slice(const struct varlena *value, int32 slicelength); /* other stuff */ -extern ToastCompressionId toast_get_compression_id(struct varlena *attr); extern char CompressionNameToMethod(const char *compression); extern const char *GetCompressionMethodName(char method); + +/* + * Extract compression ID from a varlena. + * + * Returns TOAST_INVALID_COMPRESSION_ID if the varlena is not compressed. + */ +static inline ToastCompressionId +toast_get_compression_id(struct varlena *attr) +{ + ToastCompressionId cmid = TOAST_INVALID_COMPRESSION_ID; + + /* + * If it is stored externally then fetch the compression method id from + * the external toast pointer. If compressed inline, fetch it from the + * toast compression header. + */ + if (VARATT_IS_EXTERNAL_ONDISK(attr)) + { + struct varatt_external toast_pointer; + + VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); + + if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) + cmid = VARATT_EXTERNAL_GET_COMPRESS_METHOD(toast_pointer); + } + else if (VARATT_IS_COMPRESSED(attr)) + cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); + + return cmid; +} + + + + #endif /* TOAST_COMPRESSION_H */