Re: [PATCH] SVE popcount support
Malladi, Rama <ramamalladi@hotmail.com>
From: "Malladi, Rama" <ramamalladi@hotmail.com>
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>,
Nathan Bossart <nathandbossart@gmail.com>
Date: 2024-12-04T14:51:39Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Optimize popcount functions with ARM SVE intrinsics.
- 519338ace410 18.0 landed
-
Optimize popcount functions with ARM Neon intrinsics.
- 6be53c27673a 18.0 landed
-
Rename TRY_POPCNT_FAST to TRY_POPCNT_X86_64.
- 9ac6f7e7ceb6 18.0 landed
Attachments
- SVE-popcount-implementation-comparison.png (image/png)
- SVE-popcount-support-PostgreSQL.png (image/png)
- 0002-SVE-popcount-support.patch (text/plain) patch 0002
Thank you, Kirill, for the review and the feedback. Please find inline my reply and an updated patch. On 11/29/24 2:37 AM, Kirill Reshke wrote: > PostgreSQL uses /* */ comment style. Fixed in the attached patch. > 2) > For a single line stmt PostgreSQL does not use parenthesis. Examples > [0] & [1] Fixed in the attached patch. > 3) `if (bytes > 128)` Loop in pg_popcount_sve function should be > commented. There is too much code without any comment why it works. > For example, If original source of this is some paper or other work, > we can reference it. From experimentation we found that for smaller buffer sizes, the overhead of computing prologue, kernel and epilogue loop parameters is high. So, for|< 128B|buffer size case, we use the SVE|8-bit|loop and for larger buffer sizes (|>= 128|), we use the|64-bit|SVE implementation. Attached is an SVE popcount implementation comparison. > ==== by-hand benching (I also use John Naylor's module) > > non-patched > ``` > db1=# select drive_popcount(10000000, 10000); > Time: 8886.493 ms (00:08.886) -- with small variance (+- 100ms) > db1=# select drive_popcount64(10000000, 10000); > Time: 139501.555 ms (02:19.502) with small variance (+- 1-2sec) > ``` > > patched > ``` > db1=# select drive_popcount(10000000, 10000); > Time: 8803.855 ms (00:08.804) -- with small variance > db1=# select drive_popcount64(10000000, 10000); > Time: 200716.879 ms (02:21.717) -- with small variance > ``` > > I'm not sure how to interpret these results. Looks like this does not > help much on a large $num? Can you clarify on what system, architecture did you test this patch? Note, the patch has optimizations only for `popcount` and not `popcount64`.