On 06/03/23 20:52 +0100, Jannik Glückert wrote:
we were previously only using sendfile for files smaller than 2GB, as
sendfile needs to be called repeatedly for files bigger than that.
some quick numbers, copying a 16GB file, average of 10 repetitions:
old:
real: 13.4s
user: 0.14s
sys : 7.43s
new:
real: 8.90s
user: 0.00s
sys : 3.68s
Additionally, this fixes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108178
libstdc++-v3/ChangeLog:
* acinclude.m4 (_GLIBCXX_HAVE_LSEEK): define
* config.h.in: Regenerate.
* configure: Regenerate.
* src/filesystem/ops-common.h: enable sendfile for files
>2GB in std::filesystem::copy_file, skip zero-length files
Signed-off-by: Jannik Glückert <jannik.glueck...@gmail.com>
---
libstdc++-v3/acinclude.m4 | 51 +++++----
libstdc++-v3/config.h.in | 3 +
libstdc++-v3/configure | 127 ++++++++++++++++-------
libstdc++-v3/src/filesystem/ops-common.h | 86 ++++++++-------
4 files changed, 175 insertions(+), 92 deletions(-)
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 5136c0571e8..85a09a5a869 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4583,6 +4583,7 @@ dnl _GLIBCXX_USE_FCHMOD
dnl _GLIBCXX_USE_FCHMODAT
dnl _GLIBCXX_USE_SENDFILE
dnl HAVE_LINK
+dnl HAVE_LSEEK
dnl HAVE_READLINK
dnl HAVE_SYMLINK
dnl
@@ -4718,25 +4719,6 @@ dnl
if test $glibcxx_cv_fchmodat = yes; then
AC_DEFINE(_GLIBCXX_USE_FCHMODAT, 1, [Define if fchmodat is available in
<sys/stat.h>.])
fi
-dnl
- AC_CACHE_CHECK([for sendfile that can copy files],
- glibcxx_cv_sendfile, [dnl
- case "${target_os}" in
- gnu* | linux* | solaris* | uclinux*)
- GCC_TRY_COMPILE_OR_LINK(
- [#include <sys/sendfile.h>],
- [sendfile(1, 2, (off_t*)0, sizeof 1);],
- [glibcxx_cv_sendfile=yes],
- [glibcxx_cv_sendfile=no])
- ;;
- *)
- glibcxx_cv_sendfile=no
- ;;
- esac
- ])
- if test $glibcxx_cv_sendfile = yes; then
- AC_DEFINE(_GLIBCXX_USE_SENDFILE, 1, [Define if sendfile is available in
<sys/sendfile.h>.])
- fi
dnl
AC_CACHE_CHECK([for link],
glibcxx_cv_link, [dnl
@@ -4749,6 +4731,18 @@ dnl
if test $glibcxx_cv_link = yes; then
AC_DEFINE(HAVE_LINK, 1, [Define if link is available in <unistd.h>.])
fi
+dnl
+ AC_CACHE_CHECK([for lseek],
+ glibcxx_cv_lseek, [dnl
+ GCC_TRY_COMPILE_OR_LINK(
+ [#include <unistd.h>],
+ [lseek(1, 0, SEEK_SET);],
+ [glibcxx_cv_lseek=yes],
+ [glibcxx_cv_lseek=no])
+ ])
+ if test $glibcxx_cv_lseek = yes; then
+ AC_DEFINE(HAVE_LSEEK, 1, [Define if lseek is available in <unistd.h>.])
+ fi
dnl
AC_CACHE_CHECK([for readlink],
glibcxx_cv_readlink, [dnl
@@ -4785,6 +4779,25 @@ dnl
if test $glibcxx_cv_truncate = yes; then
AC_DEFINE(HAVE_TRUNCATE, 1, [Define if truncate is available in
<unistd.h>.])
fi
+dnl
+ AC_CACHE_CHECK([for sendfile that can copy files],
+ glibcxx_cv_sendfile, [dnl
+ case "${target_os}" in
+ gnu* | linux* | solaris* | uclinux*)
+ GCC_TRY_COMPILE_OR_LINK(
+ [#include <sys/sendfile.h>],
+ [sendfile(1, 2, (off_t*)0, sizeof 1);],
+ [glibcxx_cv_sendfile=yes],
+ [glibcxx_cv_sendfile=no])
+ ;;
+ *)
+ glibcxx_cv_sendfile=no
+ ;;
+ esac
+ ])
+ if test $glibcxx_cv_sendfile = yes && test $glibcxx_cv_lseek = yes; then
+ AC_DEFINE(_GLIBCXX_USE_SENDFILE, 1, [Define if sendfile is available in
<sys/sendfile.h>.])
+ fi
dnl
AC_CACHE_CHECK([for fdopendir],
glibcxx_cv_fdopendir, [dnl
diff --git a/libstdc++-v3/src/filesystem/ops-common.h
b/libstdc++-v3/src/filesystem/ops-common.h
index abbfca43e5c..9e1b1d41dc5 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -51,6 +51,7 @@
# include <ext/stdio_filebuf.h>
# ifdef _GLIBCXX_USE_SENDFILE
# include <sys/sendfile.h> // sendfile
+# include <unistd.h> // lseek
# endif
#endif
@@ -358,6 +359,32 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
}
#ifdef NEED_DO_COPY_FILE
+#if defined _GLIBCXX_USE_SENDFILE && ! defined _GLIBCXX_FILESYSTEM_IS_WINDOWS
+ bool
+ copy_file_sendfile(int fd_in, int fd_out, size_t length) noexcept
+ {
+ // a zero-length file is either empty, or not copyable by this syscall
+ // return early to avoid the syscall cost
+ if (length == 0)
+ {
+ errno = EINVAL;
+ return false;
+ }
+ size_t bytes_left = length;
+ off_t offset = 0;
+ ssize_t bytes_copied;
+ do {
+ bytes_copied = ::sendfile(fd_out, fd_in, &offset, bytes_left);
+ bytes_left -= bytes_copied;
+ } while (bytes_left > 0 && bytes_copied > 0);
+ if (bytes_copied < 0)
+ {
+ ::lseek(fd_out, 0, SEEK_SET);
+ return false;
+ }
+ return true;
+ }
+#endif
bool
do_copy_file(const char_type* from, const char_type* to,
std::filesystem::copy_options_existing_file options,
@@ -498,28 +525,30 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
return false;
}
- size_t count = from_st->st_size;
+ bool has_copied = false;
+
#if defined _GLIBCXX_USE_SENDFILE && ! defined _GLIBCXX_FILESYSTEM_IS_WINDOWS
- off_t offset = 0;
- ssize_t n = ::sendfile(out.fd, in.fd, &offset, count);
- if (n < 0 && errno != ENOSYS && errno != EINVAL)
+ if (!has_copied)
+ has_copied = copy_file_sendfile(in.fd, out.fd, from_st->st_size);
+ if (!has_copied)
{
- ec.assign(errno, std::generic_category());
- return false;
+ if (errno != ENOSYS && errno != EINVAL)
Indentation is wrong here.
+ {
+ ec.assign(errno, std::generic_category());
+ return false;
+ }
}
- if ((size_t)n == count)
+#endif
+
+ if (has_copied)
{
- if (!out.close() || !in.close())
- {
- ec.assign(errno, std::generic_category());
- return false;
- }
- ec.clear();
- return true;
+ if (!out.close() || !in.close())
+ {
+ ec.assign(errno, std::generic_category());
Indentation is wrong here. Tabstop should be 8, so this should have
one tab followed by four spaces.
+ return false;
+ }
You need to ec.clear() before returning true here.
+ return true;
}
- else if (n > 0)
- count -= n;
-#endif // _GLIBCXX_USE_SENDFILE
using std::ios;
__gnu_cxx::stdio_filebuf<char> sbin(in.fd, ios::in|ios::binary);
@@ -530,29 +559,12 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
if (sbout.is_open())
out.fd = -1;
-#ifdef _GLIBCXX_USE_SENDFILE
- if (n != 0)
+ if (!(std::ostream(&sbout) << &sbin))
As discussed on IRC, removing the `if (count && ...)` part of this
condition breaks copying empty files, because the semantics of
ostream::operator<<(streambuf*) are:
"If the function inserts no characters, it calls setstate(failbit)"
So let's keep the original behaviour here, and deal with fixing PR
108178 in a separate patch.
{
- if (n < 0)
- n = 0;
-
- const auto p1 = sbin.pubseekoff(n, ios::beg, ios::in);
- const auto p2 = sbout.pubseekoff(n, ios::beg, ios::out);
-
- const std::streampos errpos(std::streamoff(-1));
- if (p1 == errpos || p2 == errpos)
- {
- ec = std::make_error_code(std::errc::io_error);
- return false;
- }
+ ec = std::make_error_code(std::errc::io_error);
+ return false;
Indentation is wrong here.
}
-#endif
- if (count && !(std::ostream(&sbout) << &sbin))
- {
- ec = std::make_error_code(std::errc::io_error);
- return false;
- }
if (!sbout.close() || !sbin.close())
{
ec.assign(errno, std::generic_category());
--
2.39.2
