Re: Degraded performance in cat + patch

[Pádraig Brady]

I was thinking a bit more about the patch just pushed.
It sets the buffer size to 8*st_blksize which seems a
little arbitrary, and also max buffer size is set to
32KiB even if the file system has a larger st_blksize.
I'm not sure this is desired?

How about making 32KiB the minimum as in the attached?
The patch also changes `split` and `copy` to use the
same IO size as `cat`.

cheers,
Pádraig.

>From e2791b280e255521dca10e2610ea7cec7dff11dd Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Fri, 6 Mar 2009 22:30:55 +0000
Subject: [PATCH] cat,copy,split: Set the minimum IO block size used to 32KiB

This is following on from 02c3dc9de8d3cf26b319b7a7b144878a2afb91bb
which increased the IO block size used by cat by 8 times,
but also capped it at 32KiB.
* src/system.h: Add a new OPTIMUM_BLKSIZE macro that
returns the max of ST_BLKSIZE or 32KiB, as this value
was seen as a good value for a minimum block size to use
to get good performance while minimizing system call overhead.
* src/cat.c: Use it.
* src/copy.c: ditto
* src/split.c: ditto
---
 src/cat.c    |   10 ++--------
 src/copy.c   |   13 ++-----------
 src/split.c  |    2 +-
 src/system.h |   40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/src/cat.c b/src/cat.c
index 04eb204..72837b9 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -78,12 +78,6 @@ static char *line_num_end = line_buf + LINE_COUNTER_BUF_LEN 
- 3;
 /* Preserves the `cat' function's local `newlines' between invocations.  */
 static int newlines2 = 0;
 
-static inline size_t
-compute_buffer_size (struct stat st)
-{
-  return MIN (8 * ST_BLKSIZE (st), 32 * 1024);
-}
-
 void
 usage (int status)
 {
@@ -642,7 +636,7 @@ main (int argc, char **argv)
   if (fstat (STDOUT_FILENO, &stat_buf) < 0)
     error (EXIT_FAILURE, errno, _("standard output"));
 
-  outsize = compute_buffer_size (stat_buf);
+  outsize = IO_BLKSIZE (stat_buf);
   /* Input file can be output file for non-regular files.
      fstat on pipes returns S_IFSOCK on some systems, S_IFIFO
      on others, so the checking should not be done for those types,
@@ -706,7 +700,7 @@ main (int argc, char **argv)
          ok = false;
          goto contin;
        }
-      insize = compute_buffer_size (stat_buf);
+      insize = IO_BLKSIZE (stat_buf);
 
       /* Compare the device and i-node numbers of this input file with
         the corresponding values of the (output file associated with)
diff --git a/src/copy.c b/src/copy.c
index 1918671..399abcd 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -568,7 +568,7 @@ copy_reg (char const *src_name, char const *dst_name,
     /* 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);
+    size_t buf_size = IO_BLKSIZE (sb);
 
     /* Deal with sparse files.  */
     bool last_write_made_hole = false;
@@ -596,21 +596,12 @@ copy_reg (char const *src_name, char const *dst_name,
        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,
+       size_t blcm = buffer_lcm (IO_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)
diff --git a/src/split.c b/src/split.c
index 1d8a94c..9d0f5c4 100644
--- a/src/split.c
+++ b/src/split.c
@@ -554,7 +554,7 @@ main (int argc, char **argv)
 
   if (fstat (STDIN_FILENO, &stat_buf) != 0)
     error (EXIT_FAILURE, errno, "%s", infile);
-  in_blk_size = ST_BLKSIZE (stat_buf);
+  in_blk_size = IO_BLKSIZE (stat_buf);
 
   buf = ptr_align (xmalloc (in_blk_size + 1 + page_size - 1), page_size);
 
diff --git a/src/system.h b/src/system.h
index ba74da4..b377086 100644
--- a/src/system.h
+++ b/src/system.h
@@ -218,6 +218,46 @@ enum
 # endif
 #endif
 
+/* As of Mar 6 2009 32KiB is determined to be the minimium
+ * blksize to best minimize system call overhead.
+ * This can be tested with this script with the results
+ * shown for a 1.7GHz pentium-m with 2GB of 400MHz DDR2 RAM:
+
+  for i in $(seq 0 10); do
+    size=$((8*1024**3)) #ensure this is big enough
+    bs=$((1024*2**$i))
+    printf "%7s=" $bs
+    dd bs=$bs if=/dev/zero of=/dev/null count=$(($size/$bs)) 2>&1 |
+    sed -n 's/.* \([0-9.]* [GM]B\/s\)/\1/p'
+  done
+
+     1024=734 MB/s
+     2048=1.3 GB/s
+     4096=2.4 GB/s
+     8192=3.5 GB/s
+    16384=3.9 GB/s
+    32768=5.2 GB/s
+    65536=5.3 GB/s
+   131072=5.5 GB/s
+   262144=5.7 GB/s
+   524288=5.7 GB/s
+  1048576=5.8 GB/s
+
+ * Note that this is to minimize system call overhead.
+ * Other values may be appropriate to minimize file system
+ * or disk overhead. For example on my current linux system
+ * the readahead setting is 128KiB which was read using:
+
+  file="."
+  device=$(df -P --local "$file" | tail -n1 | cut -d' ' -f1)
+  echo $(( $(blockdev --getra $device) * 512 ))
+
+ * However there isn't a portable way to get the above.
+ * In the future we could use the above method if available
+ * and default to IO_BLKSIZE if not.
+ */
+#define IO_BLKSIZE(statbuf) (MAX(32*1024, ST_BLKSIZE(statbuf)))
+
 /* Redirection and wildcarding when done by the utility itself.
    Generally a noop, but used in particular for native VMS. */
 #ifndef initialize_main
-- 
1.5.3.6