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