Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 15c9ac3

Browse files
committed
Optimize pg_readv/pg_pwritev single vector case.
For the trivial case of iovcnt == 1, kernels are measurably slower at dealing with the more complex arguments of preadv/pwritev than the equivalent plain old pread/pwrite. The overheads are worth it for iovcnt > 1, but for 1 let's just redirect to the cheaper calls. While we could leave it to callers to worry about that, we already have to have our own pg_ wrappers for portability reasons so it seems reasonable to centralize this knowledge there (thanks to Heikki for this suggestion). Try to avoid function call overheads by making them inlinable, which might also allow the compiler to avoid the branch in some cases. For systems that don't have preadv and pwritev (currently: Windows and [closed] Solaris), we might as well pull the replacement functions up into the static inline functions too. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com
1 parent a60b8a5 commit 15c9ac3

File tree

7 files changed

+74
-120
lines changed

7 files changed

+74
-120
lines changed

configure

+1-21
Original file line numberDiff line numberDiff line change
@@ -16057,7 +16057,7 @@ cat >>confdefs.h <<_ACEOF
1605716057
_ACEOF
1605816058

1605916059

16060-
# We can't use AC_REPLACE_FUNCS to replace these functions, because it
16060+
# We can't use AC_CHECK_FUNCS to detect these functions, because it
1606116061
# won't handle deployment target restrictions on macOS
1606216062
ac_fn_c_check_decl "$LINENO" "preadv" "ac_cv_have_decl_preadv" "#include <sys/uio.h>
1606316063
"
@@ -16070,16 +16070,6 @@ fi
1607016070
cat >>confdefs.h <<_ACEOF
1607116071
#define HAVE_DECL_PREADV $ac_have_decl
1607216072
_ACEOF
16073-
if test $ac_have_decl = 1; then :
16074-
16075-
else
16076-
case " $LIBOBJS " in
16077-
*" preadv.$ac_objext "* ) ;;
16078-
*) LIBOBJS="$LIBOBJS preadv.$ac_objext"
16079-
;;
16080-
esac
16081-
16082-
fi
1608316073

1608416074
ac_fn_c_check_decl "$LINENO" "pwritev" "ac_cv_have_decl_pwritev" "#include <sys/uio.h>
1608516075
"
@@ -16092,16 +16082,6 @@ fi
1609216082
cat >>confdefs.h <<_ACEOF
1609316083
#define HAVE_DECL_PWRITEV $ac_have_decl
1609416084
_ACEOF
16095-
if test $ac_have_decl = 1; then :
16096-
16097-
else
16098-
case " $LIBOBJS " in
16099-
*" pwritev.$ac_objext "* ) ;;
16100-
*) LIBOBJS="$LIBOBJS pwritev.$ac_objext"
16101-
;;
16102-
esac
16103-
16104-
fi
1610516085

1610616086

1610716087
# This is probably only present on macOS, but may as well check always

configure.ac

+3-3
Original file line numberDiff line numberDiff line change
@@ -1816,10 +1816,10 @@ AC_CHECK_DECLS(posix_fadvise, [], [], [#include <fcntl.h>])
18161816
AC_CHECK_DECLS(fdatasync, [], [], [#include <unistd.h>])
18171817
AC_CHECK_DECLS([strlcat, strlcpy, strnlen])
18181818

1819-
# We can't use AC_REPLACE_FUNCS to replace these functions, because it
1819+
# We can't use AC_CHECK_FUNCS to detect these functions, because it
18201820
# won't handle deployment target restrictions on macOS
1821-
AC_CHECK_DECLS([preadv], [], [AC_LIBOBJ(preadv)], [#include <sys/uio.h>])
1822-
AC_CHECK_DECLS([pwritev], [], [AC_LIBOBJ(pwritev)], [#include <sys/uio.h>])
1821+
AC_CHECK_DECLS([preadv], [], [], [#include <sys/uio.h>])
1822+
AC_CHECK_DECLS([pwritev], [], [], [#include <sys/uio.h>])
18231823

18241824
# This is probably only present on macOS, but may as well check always
18251825
AC_CHECK_DECLS(F_FULLFSYNC, [], [], [#include <fcntl.h>])

src/include/port/pg_iovec.h

+69-7
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include <limits.h>
1919
#include <sys/uio.h>
20+
#include <unistd.h>
2021

2122
#else
2223

@@ -36,20 +37,81 @@ struct iovec
3637
#define PG_IOV_MAX Min(IOV_MAX, 32)
3738

3839
/*
39-
* Note that pg_preadv and pg_pwritev have a pg_ prefix as a warning that the
40-
* Windows implementations have the side-effect of changing the file position.
40+
* Like preadv(), but with a prefix to remind us of a side-effect: on Windows
41+
* this changes the current file position.
4142
*/
42-
43+
static inline ssize_t
44+
pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
45+
{
4346
#if HAVE_DECL_PREADV
44-
#define pg_preadv preadv
47+
/*
48+
* Avoid a small amount of argument copying overhead in the kernel if
49+
* there is only one iovec.
50+
*/
51+
if (iovcnt == 1)
52+
return pread(fd, iov[0].iov_base, iov[0].iov_len, offset);
53+
else
54+
return preadv(fd, iov, iovcnt, offset);
4555
#else
46-
extern ssize_t pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
56+
ssize_t sum = 0;
57+
ssize_t part;
58+
59+
for (int i = 0; i < iovcnt; ++i)
60+
{
61+
part = pg_pread(fd, iov[i].iov_base, iov[i].iov_len, offset);
62+
if (part < 0)
63+
{
64+
if (i == 0)
65+
return -1;
66+
else
67+
return sum;
68+
}
69+
sum += part;
70+
offset += part;
71+
if (part < iov[i].iov_len)
72+
return sum;
73+
}
74+
return sum;
4775
#endif
76+
}
4877

78+
/*
79+
* Like pwritev(), but with a prefix to remind us of a side-effect: on Windows
80+
* this changes the current file position.
81+
*/
82+
static inline ssize_t
83+
pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
84+
{
4985
#if HAVE_DECL_PWRITEV
50-
#define pg_pwritev pwritev
86+
/*
87+
* Avoid a small amount of argument copying overhead in the kernel if
88+
* there is only one iovec.
89+
*/
90+
if (iovcnt == 1)
91+
return pwrite(fd, iov[0].iov_base, iov[0].iov_len, offset);
92+
else
93+
return pwritev(fd, iov, iovcnt, offset);
5194
#else
52-
extern ssize_t pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
95+
ssize_t sum = 0;
96+
ssize_t part;
97+
98+
for (int i = 0; i < iovcnt; ++i)
99+
{
100+
part = pg_pwrite(fd, iov[i].iov_base, iov[i].iov_len, offset);
101+
if (part < 0)
102+
{
103+
if (i == 0)
104+
return -1;
105+
else
106+
return sum;
107+
}
108+
sum += part;
109+
offset += part;
110+
if (part < iov[i].iov_len)
111+
return sum;
112+
}
113+
return sum;
53114
#endif
115+
}
54116

55117
#endif /* PG_IOVEC_H */

src/port/meson.build

-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ replace_funcs_neg = [
6666
['getpeereid'],
6767
['inet_aton'],
6868
['mkdtemp'],
69-
['preadv', 'HAVE_DECL_PREADV'],
70-
['pwritev', 'HAVE_DECL_PWRITEV'],
7169
['strlcat'],
7270
['strlcpy'],
7371
['strnlen'],

src/port/preadv.c

-43
This file was deleted.

src/port/pwritev.c

-43
This file was deleted.

src/tools/msvc/Mkvcbuild.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ sub mkvcbuild
104104
inet_net_ntop.c kill.c open.c
105105
snprintf.c strlcat.c strlcpy.c dirmod.c noblock.c path.c
106106
dirent.c getopt.c getopt_long.c
107-
preadv.c pwritev.c pg_bitutils.c
107+
pg_bitutils.c
108108
pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c
109109
pqsignal.c mkdtemp.c qsort.c qsort_arg.c bsearch_arg.c quotes.c system.c
110110
strerror.c tar.c

0 commit comments

Comments
 (0)