[EMAIL PROTECTED] (Robert Latham) writes: > That's what i thought you'd say. Ok, this patch vs. today's > CVS adds buffer-lcm.h and buffer-lcm.c, adds those files to > Makefile.am, and makes copy.c call > buffer_lcm.
That patch is a reasonable first cut, but it mishandles sparse files among other things. I installed the following instead. Thanks for prompting us to look into the problem. 2005-11-23 Paul Eggert <[EMAIL PROTECTED]> * src/copy.c: Improve performance a bit by optimizing away unnecessary system calls and going to a block size of at least 8192 (on normal hosts, anyway). This improved performance 5% on my Debian stable host (2.4.27 kernel, x86, copying from root ext3 file system to itself). Include "buffer-lcm.h". (copy_reg): Omit last argument. All callers changed. Use xmalloc to allocate rather than trusting alloca (which is unwise with large block sizes). Declare locals more locally, if possible. Use uintptr_t words instead of int words, for a bit more speed when looking for null blocks on 64-bit hosts. Optimize away reads of zero bytes on regular files. In the typical case, insist on 8 KiB buffers, at least. Avoid unnecessary extra call to fstat when checking for sparse files. Avoid now-unnecessary cast to off_t, and "0L". Avoid unnecessary test of *new_dst when checking for same owner and group. * Makefile.am (libcoreutils_a_SOURCES): Add buffer-lcm.c, buffer-lcm.h. * buffer-lcm.c, buffer-lcm.h: New files, from diffutils. Index: src/copy.c =================================================================== RCS file: /fetish/cu/src/copy.c,v retrieving revision 1.190 diff -p -u -r1.190 copy.c --- src/copy.c 25 Sep 2005 03:07:33 -0000 1.190 +++ src/copy.c 24 Nov 2005 06:40:25 -0000 @@ -31,6 +31,7 @@ #include "system.h" #include "backupfile.h" +#include "buffer-lcm.h" #include "copy.h" #include "cp-hash.h" #include "dirname.h" @@ -199,29 +200,21 @@ copy_dir (char const *src_name_in, char X provides many option settings. Return true if successful. *NEW_DST and *CHOWN_SUCCEEDED are as in copy_internal. - SRC_SB and DST_SB are the results of calling XSTAT (aka stat for - SRC_SB) on SRC_NAME and DST_NAME. */ + SRC_SB is the result of calling XSTAT (aka stat) on SRC_NAME. */ static bool copy_reg (char const *src_name, char const *dst_name, const struct cp_options *x, mode_t dst_mode, bool *new_dst, bool *chown_succeeded, - struct stat const *src_sb, - struct stat const *dst_sb) + struct stat const *src_sb) { char *buf; - size_t buf_size; - size_t buf_alignment; + char *buf_alloc = NULL; int dest_desc; int source_desc; struct stat sb; struct stat src_open_sb; - char *cp; - int *ip; bool return_val = true; - off_t n_read_total = 0; - bool last_write_made_hole = false; - bool make_holes = false; source_desc = open (src_name, O_RDONLY | O_BINARY); if (source_desc < 0) @@ -282,8 +275,6 @@ copy_reg (char const *src_name, char con goto close_src_desc; } - /* Determine the optimal buffer size. */ - if (fstat (dest_desc, &sb)) { error (0, errno, _("cannot fstat %s"), quote (dst_name)); @@ -291,126 +282,167 @@ copy_reg (char const *src_name, char con goto close_src_and_dst_desc; } - buf_size = ST_BLKSIZE (sb); - - /* Even with --sparse=always, try to create holes only - if the destination is a regular file. */ - if (x->sparse_mode == SPARSE_ALWAYS && S_ISREG (sb.st_mode)) - make_holes = true; - -#if HAVE_STRUCT_STAT_ST_BLOCKS - if (x->sparse_mode == SPARSE_AUTO && S_ISREG (sb.st_mode)) + if (! (S_ISREG (src_open_sb.st_mode) && src_open_sb.st_size == 0)) { - /* Use a heuristic to determine whether SRC_NAME contains any - sparse blocks. */ + typedef uintptr_t word; + off_t n_read_total = 0; - if (fstat (source_desc, &sb)) - { - error (0, errno, _("cannot fstat %s"), quote (src_name)); - return_val = false; - goto close_src_and_dst_desc; - } + /* Choose a suitable buffer size; it may be adjusted later. */ + size_t buf_alignment = lcm (getpagesize (), sizeof (word)); + size_t buf_alignment_slop = sizeof (word) + buf_alignment - 1; + size_t buf_size = ST_BLKSIZE (sb); + + /* Deal with sparse files. */ + bool last_write_made_hole = false; + bool make_holes = false; + + if (S_ISREG (sb.st_mode)) + { + /* Even with --sparse=always, try to create holes only + if the destination is a regular file. */ + if (x->sparse_mode == SPARSE_ALWAYS) + make_holes = true; - /* If the file has fewer blocks than would normally - be needed for a file of its size, then - at least one of the blocks in the file is a hole. */ - if (S_ISREG (sb.st_mode) - && sb.st_size / ST_NBLOCKSIZE > ST_NBLOCKS (sb)) - make_holes = true; - } +#if HAVE_STRUCT_STAT_ST_BLOCKS + /* Use a heuristic to determine whether SRC_NAME contains any sparse + blocks. If the file has fewer blocks than would normally be + needed for a file of its size, then at least one of the blocks in + the file is a hole. */ + if (x->sparse_mode == SPARSE_AUTO && S_ISREG (src_open_sb.st_mode) + && ST_NBLOCKS (src_open_sb) < src_open_sb.st_size / ST_NBLOCKSIZE) + make_holes = true; #endif + } - /* Make a buffer with space for a sentinel at the end. */ - - buf_alignment = lcm (getpagesize (), sizeof (int)); - buf = alloca (buf_size + sizeof (int) + buf_alignment - 1); - buf = ptr_align (buf, buf_alignment); + /* If not making a sparse file, try to use a more-efficient + buffer size. */ + if (! make_holes) + { + /* These days there's no point ever messing with buffers smaller + than 8 KiB. It would be nice to configure SMALL_BUF_SIZE + dynamically for this host and pair of files, but there doesn't + seem to be a good way to get readahead info portably. */ + enum { SMALL_BUF_SIZE = 8 * 1024 }; + + /* Compute the least common multiple of the input and output + buffer sizes, adjusting for outlandish values. */ + size_t blcm_max = MIN (SIZE_MAX, SSIZE_MAX) - buf_alignment_slop; + size_t blcm = buffer_lcm (ST_BLKSIZE (src_open_sb), buf_size, + blcm_max); + + /* Do not use a block size that is too small. */ + buf_size = MAX (SMALL_BUF_SIZE, blcm); + + /* Do not bother with a buffer larger than the input file, plus one + byte to make sure the file has not grown while reading it. */ + if (S_ISREG (src_open_sb.st_mode) && src_open_sb.st_size < buf_size) + buf_size = src_open_sb.st_size + 1; + + /* However, stick with a block size that is a positive multiple of + blcm, overriding the above adjustments. Watch out for + overflow. */ + buf_size += blcm - 1; + buf_size -= buf_size % blcm; + if (buf_size == 0 || blcm_max < buf_size) + buf_size = blcm; + } + + /* Make a buffer with space for a sentinel at the end. */ + buf_alloc = xmalloc (buf_size + buf_alignment_slop); + buf = ptr_align (buf_alloc, buf_alignment); - for (;;) - { - ssize_t n_read = read (source_desc, buf, buf_size); - if (n_read < 0) + for (;;) { + word *wp = NULL; + + ssize_t n_read = read (source_desc, buf, buf_size); + if (n_read < 0) + { #ifdef EINTR - if (errno == EINTR) - continue; + if (errno == EINTR) + continue; #endif - error (0, errno, _("reading %s"), quote (src_name)); - return_val = false; - goto close_src_and_dst_desc; - } - if (n_read == 0) - break; + error (0, errno, _("reading %s"), quote (src_name)); + return_val = false; + goto close_src_and_dst_desc; + } + if (n_read == 0) + break; - n_read_total += n_read; + n_read_total += n_read; - ip = NULL; - if (make_holes) - { - buf[n_read] = 1; /* Sentinel to stop loop. */ + if (make_holes) + { + char *cp; - /* Find first nonzero *word*, or the word with the sentinel. */ + buf[n_read] = 1; /* Sentinel to stop loop. */ - ip = (int *) buf; - while (*ip++ == 0) - ; + /* Find first nonzero *word*, or the word with the sentinel. */ - /* Find the first nonzero *byte*, or the sentinel. */ + wp = (word *) buf; + while (*wp++ == 0) + continue; - cp = (char *) (ip - 1); - while (*cp++ == 0) - ; + /* Find the first nonzero *byte*, or the sentinel. */ - /* If we found the sentinel, the whole input block was zero, - and we can make a hole. */ + cp = (char *) (wp - 1); + while (*cp++ == 0) + continue; + + if (cp <= buf + n_read) + /* Clear to indicate that a normal write is needed. */ + wp = NULL; + else + { + /* We found the sentinel, so the whole input block was zero. + Make a hole. */ + if (lseek (dest_desc, n_read, SEEK_CUR) < 0) + { + error (0, errno, _("cannot lseek %s"), quote (dst_name)); + return_val = false; + goto close_src_and_dst_desc; + } + last_write_made_hole = true; + } + } - if (cp > buf + n_read) + if (!wp) { - /* Make a hole. */ - if (lseek (dest_desc, (off_t) n_read, SEEK_CUR) < 0L) + size_t n = n_read; + if (full_write (dest_desc, buf, n) != n) { - error (0, errno, _("cannot lseek %s"), quote (dst_name)); + error (0, errno, _("writing %s"), quote (dst_name)); return_val = false; goto close_src_and_dst_desc; } - last_write_made_hole = true; + last_write_made_hole = false; + + /* A short read on a regular file means EOF. */ + if (n_read != buf_size && S_ISREG (src_open_sb.st_mode)) + break; } - else - /* Clear to indicate that a normal write is needed. */ - ip = NULL; } - if (ip == NULL) + + /* If the file ends with a `hole', something needs to be written at + the end. Otherwise the kernel would truncate the file at the end + of the last write operation. */ + + if (last_write_made_hole) { - size_t n = n_read; - if (full_write (dest_desc, buf, n) != n) +#if HAVE_FTRUNCATE + /* Write a null character and truncate it again. */ + if (full_write (dest_desc, "", 1) != 1 + || ftruncate (dest_desc, n_read_total) < 0) +#else + /* Seek backwards one character and write a null. */ + if (lseek (dest_desc, (off_t) -1, SEEK_CUR) < 0L + || full_write (dest_desc, "", 1) != 1) +#endif { error (0, errno, _("writing %s"), quote (dst_name)); return_val = false; goto close_src_and_dst_desc; } - last_write_made_hole = false; - } - } - - /* If the file ends with a `hole', something needs to be written at - the end. Otherwise the kernel would truncate the file at the end - of the last write operation. */ - - if (last_write_made_hole) - { -#if HAVE_FTRUNCATE - /* Write a null character and truncate it again. */ - if (full_write (dest_desc, "", 1) != 1 - || ftruncate (dest_desc, n_read_total) < 0) -#else - /* Seek backwards one character and write a null. */ - if (lseek (dest_desc, (off_t) -1, SEEK_CUR) < 0L - || full_write (dest_desc, "", 1) != 1) -#endif - { - error (0, errno, _("writing %s"), quote (dst_name)); - return_val = false; - goto close_src_and_dst_desc; } } @@ -432,8 +464,7 @@ copy_reg (char const *src_name, char con } #if HAVE_FCHOWN - if (x->preserve_ownership - && (*new_dst || !SAME_OWNER_AND_GROUP (*src_sb, *dst_sb))) + if (x->preserve_ownership && ! SAME_OWNER_AND_GROUP (*src_sb, sb)) { if (fchown (dest_desc, src_sb->st_uid, src_sb->st_gid) == 0) *chown_succeeded = true; @@ -488,6 +519,7 @@ close_src_desc: return_val = false; } + free (buf_alloc); return return_val; } @@ -1524,7 +1556,7 @@ copy_internal (char const *src_name, cha with historical practice. */ if (! copy_reg (src_name, dst_name, x, get_dest_mode (x, src_mode), &new_dst, &chown_succeeded, - &src_sb, &dst_sb)) + &src_sb)) goto un_backup; } else Index: lib/Makefile.am =================================================================== RCS file: /fetish/cu/lib/Makefile.am,v retrieving revision 1.235 diff -p -u -r1.235 Makefile.am --- lib/Makefile.am 5 Oct 2005 14:54:17 -0000 1.235 +++ lib/Makefile.am 24 Nov 2005 06:40:24 -0000 @@ -27,6 +27,7 @@ DEFS += -DLIBDIR=\"$(libdir)\" libcoreutils_a_SOURCES = \ allocsa.c allocsa.h \ + buffer-lcm.c buffer-lcm.h \ euidaccess.h \ exit.h \ fprintftime.c fprintftime.h \ --- /dev/null 2005-09-24 22:00:15.000000000 -0700 +++ lib/buffer-lcm.h 2005-11-23 15:00:03.000000000 -0800 @@ -0,0 +1,2 @@ +#include <stddef.h> +size_t buffer_lcm (size_t, size_t, size_t); --- /dev/null 2005-09-24 22:00:15.000000000 -0700 +++ lib/buffer-lcm.c 2005-11-23 15:02:09.000000000 -0800 @@ -0,0 +1,59 @@ +/* buffer-lcm.c - compute a good buffer size for dealing with two files + + Copyright (C) 2002, 2005 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2, or (at your option) + any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + +/* Written by Paul Eggert. */ + +#include "buffer-lcm.h" + +/* Return a buffer size suitable for doing I/O with files whose block + sizes are A and B. However, never return a value greater than + LCM_MAX. */ + +size_t +buffer_lcm (size_t a, size_t b, size_t lcm_max) +{ + size_t size; + + /* Use reasonable values if buffer sizes are zero. */ + if (!a) + size = b ? b : 8 * 1024; + else + { + if (b) + { + /* Return lcm (A, B) if it is in range; otherwise, fall back + on A. */ + + size_t lcm, m, n, q, r; + + /* N = gcd (A, B). */ + for (m = a, n = b; (r = m % n) != 0; m = n, n = r) + continue; + + /* LCM = lcm (A, B), if in range. */ + q = a / n; + lcm = q * b; + if (lcm <= lcm_max && lcm / b == q) + return lcm; + } + + size = a; + } + + return size <= lcm_max ? size : lcm_max; +} _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils