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

2009-10-23 Thread 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

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

2009-10-23 Thread Jim Meyering
Pádraig Brady wrote: ... 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.

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

2009-10-23 Thread Pádraig Brady
Jim Meyering wrote: Pádraig Brady wrote: ... 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]; -

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

2009-10-23 Thread Jim Meyering
Pádraig Brady wrote: Jim Meyering wrote: Pádraig Brady wrote: ... 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;

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

2009-10-23 Thread Jim Meyering
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 @@

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

2009-10-23 Thread Paolo Bonzini
+  char* buffer = malloc(BLOCKSIZE + 72); Spacing:     char *buffer = malloc (BLOCKSIZE + 72); +  if (!buffer) +    return 1; Where is that memory freed? Everything else is fine by me. Paolo

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

2009-10-23 Thread 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

Re: avoid some warnings in tests

2009-10-23 Thread Simon Josefsson
Eric Blake e...@byu.net writes: In coreutils, I turned on gcc warnings for the gnulib unit tests. This cleans up the modules that are mainly from Jim and myself, and mostly hits places that used 'main ()' or did 'char *foo = str'. Simon and Bruno had the most other tests that used 'main

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

2009-10-23 Thread Jim Meyering
Pádraig Brady wrote: Jim Meyering wrote: ... + 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

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

2009-10-23 Thread Jim Meyering
Pádraig Brady wrote: ... diff --git a/lib/copy-file.c b/lib/copy-file.c ... +enum { IO_SIZE = 32*1024 }; One more nit. Officially, we prefer to put a space on each side of every binary operator: enum { IO_SIZE = 32 * 1024 };

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

2009-10-23 Thread Pádraig Brady
Jim Meyering wrote: Pádraig Brady wrote: ... diff --git a/lib/copy-file.c b/lib/copy-file.c ... +enum { IO_SIZE = 32*1024 }; Almost there. I like the enum. Did you consider making BLOCKSIZE, below, an enum, too? as long as you're changing it... I did consider, but that would involve

getopt-gnu vs. optind=0

2009-10-23 Thread Eric Blake
The recent switch to the getopt-gnu module weakened getopt.m4 to no longer require optind=0 nor reject optreset=1 as ways to reset internal state. However, coreutils currently uses optind=0 (in at least env.c), even though I noticed that getopt.m4 and the gnulib unit tests are now careful to

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

2009-10-23 Thread Jim Meyering
Pádraig Brady wrote: Jim Meyering wrote: Pádraig Brady wrote: ... diff --git a/lib/copy-file.c b/lib/copy-file.c ... +enum { IO_SIZE = 32*1024 }; Almost there. I like the enum. Did you consider making BLOCKSIZE, below, an enum, too? as long as you're changing it... I did consider,