Re: Move pg_attribute.attcompression to earlier in struct for reduced size?

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Move pg_attribute.attcompression to earlier in struct for reduced size?
Date: 2021-06-04 22:42:57
Message-ID: 202106042242.s46x3xz5vtjw@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
inline.patch text/x-diff 2.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2021-06-04 22:47:16 Re: PG 14 release notes, first draft
Previous Message Alvaro Herrera 2021-06-04 21:52:17 Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)