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 */

Reply via email to