On 28/11/2023 14:17, Thomas Munro wrote:
> On Thu, Sep 28, 2023 at 7:33 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> + /* Avoid a slightly more expensive kernel call if there is no benefit. */
>>> + if (iovcnt == 1)
>>> + returnCode = pg_pread(vfdP->fd,
>>> + iov[0].iov_base,
>>> + iov[0].iov_len,
>>> + offset);
>>> + else
>>> + returnCode = pg_preadv(vfdP->fd, iov, iovcnt, offset);
>>
>> How about pushing down this optimization to pg_preadv() itself?
>> pg_readv() is currently just a macro if the system provides preadv(),
>> but it could be a "static inline" that does the above dance. I think
>> that optimization is platform-dependent anyway, pread() might not be any
>> faster on some OSs. In particular, if the system doesn't provide
>> preadv() and we use the implementation in src/port/preadv.c, it's the
>> same kernel call anyway.
>
> Done. I like it, I just feel a bit bad about moving the p*v()
> replacement functions around a couple of times already! I figured it
> might as well be static inline even if we use the fallback (= Solaris
> and Windows).
LGTM. I think this 0001 patch is ready for commit, independently of the
rest of the patches.
In v2-0002-Provide-vectored-variants-of-FileRead-and-FileWri-1.patch, fd.h:
> +/* Filename components */
> +#define PG_TEMP_FILES_DIR "pgsql_tmp"
> +#define PG_TEMP_FILE_PREFIX "pgsql_tmp"
> +
These seem out of place, we already have them in common/file_utils.h.
Other than that,
v2-0002-Provide-vectored-variants-of-FileRead-and-FileWri-1.patch and
v2-0003-Provide-vectored-variants-of-smgrread-and-smgrwri.patch look
good to me.
--
Heikki Linnakangas
Neon (https://neon.tech)