[PATCH] filesys.c: Use scm_sendfile to copy files.

Details

2 participants

Tomas Volf wrote 2 weeks ago

Recipients:(address . bug-guile@gnu.org)(address . ludo@gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:20250228011018.14428-1-~@wolfsden.cz

Use scm_sendfile instead of read-write loop. This moves the work into

the kernel, improving performance. This implements Ludovic's suggestion

from bug 68504.

* libguile/filesys.c (scm_copy_file2): Use scm_sendfile.

---

libguile/filesys.c | 21 +++++++++++++--------

1 file changed, 13 insertions(+), 8 deletions(-)

Toggle diff (41 lines)diff --git a/libguile/filesys.c b/libguile/filesys.c
index 3bfa5eb91..8a05f066f 100644
--- a/libguile/filesys.c
+++ b/libguile/filesys.c
@@ -1,7 +1,7 @@
 /* Copyright 1996-2002,2004,2006,2009-2019,2021
      Free Software Foundation, Inc.
    Copyright 2021 Maxime Devos <maximedevos@telenet.be>
-   Copyright 2024 Tomas Volf <~@wolfsden.cz>
+   Copyright 2024, 2025 Tomas Volf <~@wolfsden.cz>
 
    This file is part of Guile.
 
@@ -1354,13 +1354,18 @@ SCM_DEFINE (scm_copy_file2, "copy-file", 2, 0, 1,
     scm_syserror ("copy-file: copy-on-write failed");
 
   if (clone_res)
-    while ((n = read (oldfd, buf, sizeof buf)) > 0)
-      if (write (newfd, buf, n) != n)
-        {
-          close (oldfd);
-          close (newfd);
-          SCM_SYSERROR;
-        }
+    {
+      off_t end;
+      if ((end = lseek_or_lseek64 (oldfd, 0, SEEK_END)) < 0)
+        SCM_SYSERROR;
+      if (lseek_or_lseek64 (oldfd, 0, SEEK_SET) < 0)
+        SCM_SYSERROR;
+
+      scm_sendfile (scm_from_int (newfd),
+                    scm_from_int (oldfd),
+                    scm_from_off_t (end),
+                    SCM_UNDEFINED);
+    }
   close (oldfd);
   if (close (newfd) == -1)
     SCM_SYSERROR;
-- 
2.48.1

Ludovic Courtès wrote 2 weeks ago

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)(address . 76623@debbugs.gnu.org)

Message-ID:87h64drfs1.fsf@gnu.org

Hi,

Tomas Volf <~@wolfsden.cz> skribis:

Toggle quote (6 lines)

> Use scm_sendfile instead of read-write loop. This moves the work into

> the kernel, improving performance. This implements Ludovic's suggestion

> from bug 68504.

> * libguile/filesys.c (scm_copy_file2): Use scm_sendfile.

Nice! I had to apply the changes below to appease GCC.

The patch LGTM but I realize there’s no real ‘copy-file’ test. Not your

fault but would you mind adding one or two tests?

Thanks,

Ludo’.

Toggle diff (16 lines)diff --git a/libguile/filesys.c b/libguile/filesys.c
index 00171dade..4f861ab35 100644
--- a/libguile/filesys.c
+++ b/libguile/filesys.c
@@ -1306,10 +1306,9 @@ SCM_DEFINE (scm_copy_file2, "copy-file", 2, 0, 1,
 {
   char *c_oldfile, *c_newfile;
   int oldfd, newfd;
-  int n, rv;
+  int rv;
   SCM cow = sym_auto;
   int clone_res;
-  char buf[BUFSIZ];
   struct stat_or_stat64 oldstat;
 
   scm_dynwind_begin (0);

Your comment

Commenting via the web interface is currently disabled.

To respond to this issue using the mumi CLI, first switch to it

mumi current 76623

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags

mumi command -t +confirmed -t +easy

Or, remove the moreinfo tag and set the help tag

mumi command -t -moreinfo -t +help