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 →
  1. Optimize popcount functions with ARM SVE intrinsics.

  2. Optimize popcount functions with ARM Neon intrinsics.

  3. Rename TRY_POPCNT_FAST to TRY_POPCNT_X86_64.

Attachments

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`.