| From: | John Naylor <johncnaylorls(at)gmail(dot)com> |
|---|---|
| To: | Andrew Kim <tenistarkim(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Oleg Tselebrovskiy <o(dot)tselebrovskiy(at)postgrespro(dot)ru> |
| Subject: | Re: Proposal for enabling auto-vectorization for checksum calculations |
| Date: | 2026-01-11 23:19:18 |
| Message-ID: | CANWCAZYg2MVbYTaczNYNC2kaPodtfB8toUfE2Mhp9kut=2wzEA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Nov 6, 2025 at 6:50 AM Andrew Kim <tenistarkim(at)gmail(dot)com> wrote:
> The v9 patch series is attached.
Sorry for the delay. I found some issues last month and needed to
consider the tradeoffs.
First, apparently it has gone unnoticed by everyone, myself included,
that no version has passed Meson CI since v6:
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F5726
That's because `ninja -C build -t missingdeps` gives:
Missing dep: src/port/libpgport_shlib_checksum.a.p/checksum.c.o uses
src/include/utils/errcodes.h (generated by CUSTOM_COMMAND)
Missing dep: src/port/libpgport_checksum.a.p/checksum.c.o uses
src/include/utils/errcodes.h (generated by CUSTOM_COMMAND)
Processed 2561 nodes.
Error: There are 2 missing dependency paths.
2 targets had depfile dependencies on 1 distinct generated inputs
(from 1 rules) without a non-depfile dep path to the generator.
There might be build flakiness if any of the targets listed above are
built alone, or not late enough, in a clean output directory.
In the back of my mind I was worried of consequences of something in
src/port depending on backend types, but hadn't seen any in my local
builds. It seems the proximate cause is the removal of this stanza
with no equivalent replacement:
--- a/src/backend/storage/page/meson.build
+++ b/src/backend/storage/page/meson.build
@@ -1,14 +1,5 @@
# Copyright (c) 2022-2025, PostgreSQL Global Development Group
-checksum_backend_lib = static_library('checksum_backend_lib',
- 'checksum.c',
- dependencies: backend_build_deps,
- kwargs: internal_lib_args,
- c_args: vectorize_cflags + unroll_loops_cflags,
-)
-
-backend_link_with += checksum_backend_lib
The low-level algorithm doesn't care about database pages, only
integers, so first I tried to surgically isolate the concepts, but
that was too messy.
In the attached v10-0003, I went back to something more similar to v6,
but incorporated Andrew's idea of using PG_CHECKSUM_INTERNAL to allow
for flexibility. Now pg_filedump compiles without any changes, so
that's a plus.
> - Provides public interfaces wrapping the basic implementation
> - No code duplication (checksum.c includes checksum_impl.h)
Upthread I mentioned "thin wrappers", but so far I haven't seen it in
any patch versions, so I don't think this term means the same thing to
you as it does to me (I saw pretty clear duplication in v9). It then
occurred to me that with function attribute targets, doing the naive
thing throws a compiler error IIRC -- namely just have a notional
function call that then gets inlined and re-targeted. So in v10 I
separated the body of checksum_block to a semi-private header to
provide hardware-specific definitions for core code, while also
maintaining the same one that external code expects.
For this to be commitable, I think (and I think Oleg agrees) that the
feature detection should go in src/port. Some of us have been thinking
of refactoring and centralizing the feature detection, and now may be
a good time to do it. Before going that far, I wanted to see what
people think of v10.
--
John Naylor
Amazon Web Services
| Attachment | Content-Type | Size |
|---|---|---|
| v10-0003-Enable-autovectorizing-pg_checksum_block-with-AV.patch | application/x-patch | 13.8 KB |
| v10-0002-Adjust-benchmark-to-use-core-checksum.patch | application/x-patch | 1.6 KB |
| v10-0001-Benchmark-code-for-postgres-checksums.patch | application/x-patch | 5.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-01-11 23:23:18 | Re: Use correct macro for accessing offset numbers. |
| Previous Message | Tomas Vondra | 2026-01-11 22:20:46 | Re: Parallel CREATE INDEX for GIN indexes |