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

[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, but that would involve changing to
> use the compiler to verify() it was a multiple of 64.
> I thought that a little invasive and should be part of
> a general refactoring that seems needed on those modules.
>
>>> diff --git a/lib/md2.c b/lib/md2.c
>> ...
>>> +  char* buffer = malloc (BLOCKSIZE + 72);
>>
>> Spacing again (search for 'char*'; there are more):
>>
>>      char *buffer = malloc (BLOCKSIZE + 72);
>
> Stupid muscle memory.
> Attached.
...
> 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

Thanks.  I applied that with git am to the
change set 3 or 4 down the stack, rebased it to the top,
and tested with this:

  ./gnulib-tool --create-testdir --with-tests --test crypto/md2 crypto/md4 \
      crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512

It passed, so I pushed it.