Re: [PATCH] md5: accepts a new --threads option

[Pádraig Brady]

Paolo Bonzini wrote:
> On 10/22/2009 01:09 PM, Pádraig Brady wrote:
>> Jim Meyering wrote:
>>> Pádraig Brady wrote:
>>>
>>>> p.s. I'll look at bypassing stdio on input to see
>>>> if I can get at least the 2% back
>>>
>>> IMHO, even if it did, it would not be worth it.
>>
>> Right, a quick test here shows only a 0.8% gain from
>> bypassing stdio. However I also noticed that the
>> digest routines in gnulib do fread(4096).
>> Bumping that up to 32KiB gives a 3% boost.
>> That would be increased even more if more efficient
>> checksumming routines are used (which is on the horizon).
>> Note the coreutils min was bumped to 32KiB recently:
>> http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=55efc5f
>>
>> Does anyone have any objections to increasing
>> the stack requirement by 28672 bytes?
> 
> I do.  It's common for threads to not have a >32 kb thread.  What I
> don't object to, is mallocing the buffer. :-)

Thanks Paolo,

How about the attached.
Per byte it performs the same as the stack method,
per file there is a small overhead.

cheers,
Pádraig.

>From eeff999a050eb225a5fa2f47509993785d3c3e74 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 22 Oct 2009 17:36:28 +0100
Subject: [PATCH] digests, copy-file: increase the IO buffer size from 4KiB to 
32KiB

This results in a significant decrease in syscall overhead
giving a 3% speedup to the digest utilities for example
(when processing large files from cache).
Storage is moved from the stack to the heap as some
threaded environments for example can have small stacks.

* lib/copy-file.c (copy_file_preserving): Use a 32KiB malloced buffer
* modules/copy-file: Depend on xalloc
* lib/md2.c: Likewise
* lib/md4.c: Likewise
* lib/md5.c: Likewise
* lib/sha1.c: Likewise
* lib/sha256.c: Likewise
* lib/sha512.c: Likewise
---
 ChangeLog         |   11 +++++++++++
 lib/copy-file.c   |    7 ++++---
 lib/md2.c         |    7 +++++--
 lib/md4.c         |    7 +++++--
 lib/md5.c         |    7 +++++--
 lib/sha1.c        |    7 +++++--
 lib/sha256.c      |    7 +++++--
 lib/sha512.c      |    7 +++++--
 modules/copy-file |    1 +
 9 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3271366..f320e99 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2009-10-22  Pádraig Brady  <address@hidden>
+
+       Use a better IO block size for modern systems
+       * lib/copy-file.c (copy_file_preserving): Used a 32KiB malloced buffer.
+       * lib/md2.c: Likewise.
+       * lib/md4.c: Likewise.
+       * lib/md5.c: Likewise.
+       * lib/sha1.c: Likewise.
+       * lib/sha256.c: Likewise.
+       * lib/sha512.c: Likewise.
+
 2009-10-20  Eric Blake  <address@hidden>
 
        utimensat: work around Solaris 9 bug
diff --git a/lib/copy-file.c b/lib/copy-file.c
index 1587905..c08fe47 100644
--- a/lib/copy-file.c
+++ b/lib/copy-file.c
@@ -42,6 +42,7 @@
 #include "acl.h"
 #include "binary-io.h"
 #include "gettext.h"
+#include "xalloc.h"
 
 #define _(str) gettext (str)
 
@@ -50,6 +51,7 @@
 #undef open
 #undef close
 
+enum { IO_SIZE = 32*1024 };
 
 void
 copy_file_preserving (const char *src_filename, const char *dest_filename)
@@ -58,8 +60,7 @@ copy_file_preserving (const char *src_filename, const char 
*dest_filename)
   struct stat statbuf;
   int mode;
   int dest_fd;
-  char buf[4096];
-  const size_t buf_size = sizeof (buf);
+  char *buf = xmalloc (IO_SIZE);
 
   src_fd = open (src_filename, O_RDONLY | O_BINARY);
   if (src_fd < 0 || fstat (src_fd, &statbuf) < 0)
@@ -76,7 +77,7 @@ copy_file_preserving (const char *src_filename, const char 
*dest_filename)
   /* Copy the file contents.  */
   for (;;)
     {
-      size_t n_read = safe_read (src_fd, buf, buf_size);
+      size_t n_read = safe_read (src_fd, buf, IO_SIZE);
       if (n_read == SAFE_READ_ERROR)
        error (EXIT_FAILURE, errno, _("error reading \"%s\""), src_filename);
       if (n_read == 0)
diff --git a/lib/md2.c b/lib/md2.c
index cb4c63b..f8878c0 100644
--- a/lib/md2.c
+++ b/lib/md2.c
@@ -33,7 +33,7 @@
 # include "unlocked-io.h"
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -94,9 +94,12 @@ int
 md2_stream (FILE *stream, void *resblock)
 {
   struct md2_ctx ctx;
-  char buffer[BLOCKSIZE + 72];
   size_t sum;
 
+  char* buffer = malloc(BLOCKSIZE + 72);
+  if (!buffer)
+    return 1;
+
   /* Initialize the computation context.  */
   md2_init_ctx (&ctx);
 
diff --git a/lib/md4.c b/lib/md4.c
index e348417..8f7c741 100644
--- a/lib/md4.c
+++ b/lib/md4.c
@@ -40,7 +40,7 @@
 # define SWAP(n) (n)
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -122,9 +122,12 @@ int
 md4_stream (FILE * stream, void *resblock)
 {
   struct md4_ctx ctx;
-  char buffer[BLOCKSIZE + 72];
   size_t sum;
 
+  char* buffer = malloc(BLOCKSIZE + 72);
+  if (!buffer)
+    return 1;
+
   /* Initialize the computation context.  */
   md4_init_ctx (&ctx);
 
diff --git a/lib/md5.c b/lib/md5.c
index 2213dc1..5445701 100644
--- a/lib/md5.c
+++ b/lib/md5.c
@@ -56,7 +56,7 @@
 # define SWAP(n) (n)
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -136,9 +136,12 @@ int
 md5_stream (FILE *stream, void *resblock)
 {
   struct md5_ctx ctx;
-  char buffer[BLOCKSIZE + 72];
   size_t sum;
 
+  char* buffer = malloc(BLOCKSIZE + 72);
+  if (!buffer)
+    return 1;
+
   /* Initialize the computation context.  */
   md5_init_ctx (&ctx);
 
diff --git a/lib/sha1.c b/lib/sha1.c
index 9c6c7ae..cd8b876 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -41,7 +41,7 @@
     (((n) << 24) | (((n) & 0xff00) << 8) | (((n) >> 8) & 0xff00) | ((n) >> 24))
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -124,9 +124,12 @@ int
 sha1_stream (FILE *stream, void *resblock)
 {
   struct sha1_ctx ctx;
-  char buffer[BLOCKSIZE + 72];
   size_t sum;
 
+  char* buffer = malloc(BLOCKSIZE + 72);
+  if (!buffer)
+    return 1;
+
   /* Initialize the computation context.  */
   sha1_init_ctx (&ctx);
 
diff --git a/lib/sha256.c b/lib/sha256.c
index 0ad9444..34b2c7d 100644
--- a/lib/sha256.c
+++ b/lib/sha256.c
@@ -38,7 +38,7 @@
     (((n) << 24) | (((n) & 0xff00) << 8) | (((n) >> 8) & 0xff00) | ((n) >> 24))
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -169,9 +169,12 @@ int
 sha256_stream (FILE *stream, void *resblock)
 {
   struct sha256_ctx ctx;
-  char buffer[BLOCKSIZE + 72];
   size_t sum;
 
+  char* buffer = malloc(BLOCKSIZE + 72);
+  if (!buffer)
+    return 1;
+
   /* Initialize the computation context.  */
   sha256_init_ctx (&ctx);
 
diff --git a/lib/sha512.c b/lib/sha512.c
index 261a7bb..2192328 100644
--- a/lib/sha512.c
+++ b/lib/sha512.c
@@ -45,7 +45,7 @@
                         u64shr (n, 56))))
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 128 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -177,9 +177,12 @@ int
 sha512_stream (FILE *stream, void *resblock)
 {
   struct sha512_ctx ctx;
-  char buffer[BLOCKSIZE + 72];
   size_t sum;
 
+  char* buffer = malloc(BLOCKSIZE + 72);
+  if (!buffer)
+    return 1;
+
   /* Initialize the computation context.  */
   sha512_init_ctx (&ctx);
 
diff --git a/modules/copy-file b/modules/copy-file
index 5f1c6e9..6941e5a 100644
--- a/modules/copy-file
+++ b/modules/copy-file
@@ -16,6 +16,7 @@ gettext-h
 open
 safe-read
 unistd
+xalloc
 
 configure.ac:
 gl_COPY_FILE
-- 
1.6.2.5