Simplify replacement code for preadv and pwritev.
authorThomas Munro <tmunro@postgresql.org>
Thu, 4 Aug 2022 23:43:14 +0000 (11:43 +1200)
committerThomas Munro <tmunro@postgresql.org>
Fri, 5 Aug 2022 02:04:02 +0000 (14:04 +1200)
preadv() and pwritev() are not standardized by POSIX, but appeared in
NetBSD in 1999 and were adopted by at least OpenBSD, FreeBSD,
DragonFlyBSD, Linux, AIX, illumos and macOS.  We don't use them much
yet, but an active proposal uses them heavily.

In 15, we had two replacement implementations for other OSes: one based
on lseek() + -v function if available for true vector I/O, and the other
based on a loop over p- function.

The former would be an obstacle to hypothetical future multi-threaded
code sharing file descriptors, while the latter would not, since commit
cf112c12.  Furthermore, the number of targeted systems that could
benefit from the former's potential upside has dwindled to just one
niche OS, since macOS added the functions and we de-supported HP-UX.
That doesn't seem like a good trade-off.

Therefore, drop the lseek()-based variant, and also the pg_ prefix now
that the file position portability hazard is gone.

At the time of writing, the only systems in our build farm that lack
native preadv/pwritev and thus use fallback code are:

 * Solaris (but not illumos)
 * macOS before release 11.0
 * Windows

With this commit, the above systems will now use the *same* fallback
code, the version that loops over pread()/pwrite().  Windows already
used that (though a later proposal may include true vector I/O for
Windows), so this decision really only affects Solaris, until it gets
around to adding these system calls.

Also remove some useless includes while here.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com

configure
configure.ac
src/backend/storage/file/fd.c
src/include/pg_config.h.in
src/include/port/pg_iovec.h
src/port/preadv.c
src/port/pwritev.c
src/tools/msvc/Solution.pm

index 87b3753846c0c61a77c8457b6770a2241a188c37..b7a77033586a73a21b141ac012ee2ba5845c8886 100755 (executable)
--- a/configure
+++ b/configure
@@ -16039,7 +16039,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in backtrace_symbols clock_gettime copyfile fdatasync getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np readv setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l writev
+for ac_func in backtrace_symbols clock_gettime copyfile fdatasync getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
index cd29d533a5918e7a8a5e0f20b3bd6ce6edd83504..2938c9c7de70eb3d08f6a0fc84e57da8f6caaab7 100644 (file)
@@ -1803,7 +1803,6 @@ AC_CHECK_FUNCS(m4_normalize([
    posix_fallocate
    ppoll
    pthread_is_threaded_np
-   readv
    setproctitle
    setproctitle_fast
    strchrnul
@@ -1812,7 +1811,6 @@ AC_CHECK_FUNCS(m4_normalize([
    sync_file_range
    uselocale
    wcstombs_l
-   writev
 ]))
 
 # These typically are compiler builtins, for which AC_CHECK_FUNCS fails.
index 3d1a97335495aebc67cd47a2485fc945bd117004..5a2eb01238174931e329aa4b44352ccdc7659a95 100644 (file)
@@ -3762,7 +3762,7 @@ data_sync_elevel(int elevel)
 }
 
 /*
- * A convenience wrapper for pg_pwritev() that retries on partial write.  If an
+ * A convenience wrapper for pwritev() that retries on partial write.  If an
  * error is returned, it is unspecified how much has been written.
  */
 ssize_t
@@ -3782,7 +3782,7 @@ pg_pwritev_with_retry(int fd, const struct iovec *iov, int iovcnt, off_t offset)
    for (;;)
    {
        /* Write as much as we can. */
-       part = pg_pwritev(fd, iov, iovcnt, offset);
+       part = pwritev(fd, iov, iovcnt, offset);
        if (part < 0)
            return -1;
 
index 4d61ecd91429332c0776fd7069479fb913762a15..dfee47a1a45ebe73a04b2664bbc1ac832aadedc2 100644 (file)
 /* Define to 1 if you have the <readline/readline.h> header file. */
 #undef HAVE_READLINE_READLINE_H
 
-/* Define to 1 if you have the `readv' function. */
-#undef HAVE_READV
-
 /* Define to 1 if you have the `rl_completion_matches' function. */
 #undef HAVE_RL_COMPLETION_MATCHES
 
 /* Define to 1 if you have the <winldap.h> header file. */
 #undef HAVE_WINLDAP_H
 
-/* Define to 1 if you have the `writev' function. */
-#undef HAVE_WRITEV
-
 /* Define to 1 if you have the `X509_get_signature_nid' function. */
 #undef HAVE_X509_GET_SIGNATURE_NID
 
index f0b1a71bcb83b090a6999b4ceedcd0fa859db848..f0a50c0e015797c4888e804ac1e9de0ce2597c6a 100644 (file)
@@ -39,16 +39,12 @@ struct iovec
 /* Define a reasonable maximum that is safe to use on the stack. */
 #define PG_IOV_MAX Min(IOV_MAX, 32)
 
-#if HAVE_DECL_PREADV
-#define pg_preadv preadv
-#else
-extern ssize_t pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+#if !HAVE_DECL_PREADV
+extern ssize_t preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
 #endif
 
-#if HAVE_DECL_PWRITEV
-#define pg_pwritev pwritev
-#else
-extern ssize_t pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+#if !HAVE_DECL_PWRITEV
+extern ssize_t pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
 #endif
 
 #endif                         /* PG_IOVEC_H */
index aa7537503fb1bde3296e76ab2be997c35b220a49..48b64d4593d778349da4a5591048bf7997999652 100644 (file)
@@ -8,33 +8,19 @@
  * IDENTIFICATION
  *   src/port/preadv.c
  *
- * Note that this implementation changes the current file position, unlike
- * the POSIX-like function, so we use the name pg_preadv().
- *
  *-------------------------------------------------------------------------
  */
 
 
 #include "postgres.h"
 
-#ifdef WIN32
-#include <windows.h>
-#else
 #include <unistd.h>
-#endif
 
 #include "port/pg_iovec.h"
 
 ssize_t
-pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 {
-#ifdef HAVE_READV
-   if (iovcnt == 1)
-       return pread(fd, iov[0].iov_base, iov[0].iov_len, offset);
-   if (lseek(fd, offset, SEEK_SET) < 0)
-       return -1;
-   return readv(fd, iov, iovcnt);
-#else
    ssize_t     sum = 0;
    ssize_t     part;
 
@@ -54,5 +40,4 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
            return sum;
    }
    return sum;
-#endif
 }
index cb7421381e4d7b67b6e3ac3003f2d8ba33062ad0..8b303fcbcdc0e4fe8b0961feda4f011cf9c8f842 100644 (file)
@@ -8,33 +8,19 @@
  * IDENTIFICATION
  *   src/port/pwritev.c
  *
- * Note that this implementation changes the current file position, unlike
- * the POSIX-like function, so we use the name pg_pwritev().
- *
  *-------------------------------------------------------------------------
  */
 
 
 #include "postgres.h"
 
-#ifdef WIN32
-#include <windows.h>
-#else
 #include <unistd.h>
-#endif
 
 #include "port/pg_iovec.h"
 
 ssize_t
-pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 {
-#ifdef HAVE_WRITEV
-   if (iovcnt == 1)
-       return pwrite(fd, iov[0].iov_base, iov[0].iov_len, offset);
-   if (lseek(fd, offset, SEEK_SET) < 0)
-       return -1;
-   return writev(fd, iov, iovcnt);
-#else
    ssize_t     sum = 0;
    ssize_t     part;
 
@@ -54,5 +40,4 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
            return sum;
    }
    return sum;
-#endif
 }
index 5a461a5212c3b8dee131c03a31f6ad3fb1acb8ac..dd9d1a58ad60bb54231a538894770eb9526a9534 100644 (file)
@@ -332,7 +332,6 @@ sub GenerateFiles
        HAVE_READLINE_H             => undef,
        HAVE_READLINE_HISTORY_H     => undef,
        HAVE_READLINE_READLINE_H    => undef,
-       HAVE_READV                  => undef,
        HAVE_RL_COMPLETION_MATCHES  => undef,
        HAVE_RL_COMPLETION_SUPPRESS_QUOTE        => undef,
        HAVE_RL_FILENAME_COMPLETION_FUNCTION     => undef,
@@ -408,7 +407,6 @@ sub GenerateFiles
        HAVE_WINLDAP_H                           => undef,
        HAVE_WCSTOMBS_L                          => 1,
        HAVE_VISIBILITY_ATTRIBUTE                => undef,
-       HAVE_WRITEV                              => undef,
        HAVE_X509_GET_SIGNATURE_NID              => 1,
        HAVE_X86_64_POPCNTQ                      => undef,
        HAVE__BOOL                               => undef,