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

[Pádraig Brady]

Jim Meyering wrote:
> Pádraig Brady wrote:
>> 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);
> 
> Spacing:
> 
>      char *buffer = malloc (BLOCKSIZE + 72);
> 
>> +  if (!buffer)
>> +    return 1;
> 
> Where is that memory freed?

LOL. <blush!!>

I also didn't include stdlib.h
I also forgot to update sha{224,384}_stream()
I also messed up the tabbing

Hopefully last version attached.

cheers,
Pádraig.

>From 1254d563eb254caf48d0967af379493a02a30db0 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   |    9 ++++++---
 lib/md2.c         |   14 +++++++++++---
 lib/md4.c         |   14 +++++++++++---
 lib/md5.c         |   13 ++++++++++---
 lib/sha1.c        |   14 +++++++++++---
 lib/sha256.c      |   25 ++++++++++++++++++++-----
 lib/sha512.c      |   25 ++++++++++++++++++++-----
 modules/copy-file |    1 +
 9 files changed, 101 insertions(+), 25 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..4d9b86a 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)
@@ -86,6 +87,8 @@ copy_file_preserving (const char *src_filename, const char 
*dest_filename)
        error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
     }
 
+  free (buf);
+
 #if !USE_ACL
   if (close (dest_fd) < 0)
     error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
diff --git a/lib/md2.c b/lib/md2.c
index cb4c63b..cf070ec 100644
--- a/lib/md2.c
+++ b/lib/md2.c
@@ -24,6 +24,7 @@
 
 #include "md2.h"
 
+#include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
 
@@ -33,7 +34,7 @@
 # include "unlocked-io.h"
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -94,9 +95,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);
 
@@ -125,7 +129,10 @@ md2_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -150,6 +157,7 @@ process_partial_block:;
 
   /* Construct result in desired memory.  */
   md2_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
diff --git a/lib/md4.c b/lib/md4.c
index e348417..6813384 100644
--- a/lib/md4.c
+++ b/lib/md4.c
@@ -26,6 +26,7 @@
 
 #include <stddef.h>
 #include <stdlib.h>
+#include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
 
@@ -40,7 +41,7 @@
 # define SWAP(n) (n)
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 64 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -122,9 +123,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);
 
@@ -153,7 +157,10 @@ md4_stream (FILE * stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -178,6 +185,7 @@ process_partial_block:;
 
   /* Construct result in desired memory.  */
   md4_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
diff --git a/lib/md5.c b/lib/md5.c
index 2213dc1..043eb09 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);
 
@@ -167,7 +170,10 @@ md5_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -192,6 +198,7 @@ process_partial_block:
 
   /* Construct result in desired memory.  */
   md5_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
diff --git a/lib/sha1.c b/lib/sha1.c
index 9c6c7ae..b63dbf0 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -28,6 +28,7 @@
 #include "sha1.h"
 
 #include <stddef.h>
+#include <stdlib.h>
 #include <string.h>
 
 #if USE_UNLOCKED_IO
@@ -41,7 +42,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 +125,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);
 
@@ -155,7 +159,10 @@ sha1_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -180,6 +187,7 @@ sha1_stream (FILE *stream, void *resblock)
 
   /* Construct result in desired memory.  */
   sha1_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
diff --git a/lib/sha256.c b/lib/sha256.c
index 0ad9444..a76a5d1 100644
--- a/lib/sha256.c
+++ b/lib/sha256.c
@@ -25,6 +25,7 @@
 #include "sha256.h"
 
 #include <stddef.h>
+#include <stdlib.h>
 #include <string.h>
 
 #if USE_UNLOCKED_IO
@@ -38,7 +39,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 +170,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);
 
@@ -200,7 +204,10 @@ sha256_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -225,6 +232,7 @@ sha256_stream (FILE *stream, void *resblock)
 
   /* Construct result in desired memory.  */
   sha256_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
@@ -233,9 +241,12 @@ int
 sha224_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.  */
   sha224_init_ctx (&ctx);
 
@@ -264,7 +275,10 @@ sha224_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -289,6 +303,7 @@ sha224_stream (FILE *stream, void *resblock)
 
   /* Construct result in desired memory.  */
   sha224_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
diff --git a/lib/sha512.c b/lib/sha512.c
index 261a7bb..62c53a8 100644
--- a/lib/sha512.c
+++ b/lib/sha512.c
@@ -25,6 +25,7 @@
 #include "sha512.h"
 
 #include <stddef.h>
+#include <stdlib.h>
 #include <string.h>
 
 #if USE_UNLOCKED_IO
@@ -45,7 +46,7 @@
                         u64shr (n, 56))))
 #endif
 
-#define BLOCKSIZE 4096
+#define BLOCKSIZE 32768
 #if BLOCKSIZE % 128 != 0
 # error "invalid BLOCKSIZE"
 #endif
@@ -177,9 +178,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);
 
@@ -208,7 +212,10 @@ sha512_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -233,6 +240,7 @@ sha512_stream (FILE *stream, void *resblock)
 
   /* Construct result in desired memory.  */
   sha512_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
@@ -241,9 +249,12 @@ int
 sha384_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.  */
   sha384_init_ctx (&ctx);
 
@@ -272,7 +283,10 @@ sha384_stream (FILE *stream, void *resblock)
                 exit the loop after a partial read due to e.g., EAGAIN
                 or EWOULDBLOCK.  */
              if (ferror (stream))
-               return 1;
+               {
+                 free (buffer);
+                 return 1;
+               }
              goto process_partial_block;
            }
 
@@ -297,6 +311,7 @@ sha384_stream (FILE *stream, void *resblock)
 
   /* Construct result in desired memory.  */
   sha384_finish_ctx (&ctx, resblock);
+  free (buffer);
   return 0;
 }
 
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