Re: [Monotone-devel] Speedup chances

[Jack Lloyd]

On Thu, May 08, 2008 at 09:48:14AM -0700, Eric Anderson wrote:
> Markus Schiltknecht writes:
>  > Eric Anderson wrote:
>  > > Revision: 464e510af4959231ff63352c902c689b0f1687aa
>  > > Branch: net.venge.monotone.experiment.performance
>  > 
>  > Hm.. why didn't any of this get merged into mainline? Looks like there 
>  > are more good ideas lying around in that branch.
> 
> IIRC the feeling was that many of the fixes were somewhat ugly and
> people thought this style of performance improvements didn't really
> matter that much.  They would rather have had algorithmic performance
> improvements.  (Consider the pthread hack, 20% performance
> improvement, grotesque code).

Speaking of 'interesting' optimization ideas with potentially nasty
results on the code, something I thought about a while back:

Currently calculate_ident() returns the hash value immediately. If
there was a way to instead return a promise of a hash, that could be
forced to resolve when the value was actually needed, one could do
some reasonably interesting optimizations (thread pools (what is the
'pthread hack'? is that basically this idea?), or a SIMD SHA-1 to do 4
independent SHA computations in parallel with SSE2/Altivec, probably
other optimizations are possible when batching is feasible). I never
wrote any code for it. I also did not investigate if it would actually
help... perhaps at the point calculate_ident is called, it is too late
to extract much parallelism b/c the value is needed right away, I
don't know.

Somewhat less exotic, but perhaps the appended patch would improve
performance a bit? It builds and seems to work on Gentoo/amd64, but I
don't know how much if at all it would improve performance and don't
have a good way to measure. Clearly it is somewhat less generic and
more verbose than the previous code, but if it helps, it helps...
Ran the test suite and got:

Of 483 tests run:
        446 succeeded
        1 failed
        36 had expected failures
        0 succeeded unexpectedly
        0 were skipped

I don't know if that one failure was that I broke something or if it
is preexistingly broken in n.v.m. I'm too lazy to bother rerunning the
tests without the patch applies since I assume if I broke
calculate_ident the tests would fail pretty hard.

-Jack


#
# old_revision [25d76182286bccd52c355d6770414c62065c823b]
#
# patch "transforms.cc"
#  from [344c1f23d33145b51f0fc500014a75da95b3c115]
#    to [b7ad4e46ca43970a1f027e711f2de7206f8eade6]
#
============================================================
--- transforms.cc       344c1f23d33145b51f0fc500014a75da95b3c115
+++ transforms.cc       b7ad4e46ca43970a1f027e711f2de7206f8eade6
@@ -176,9 +176,16 @@ calculate_ident(data const & dat,
 {
   try
     {
-      static cached_botan_pipe p(new Pipe(new Hash_Filter("SHA-160")));
-      p->process_msg(dat());
-      ident = id(p->read_all_as_string(Pipe::LAST_MESSAGE));
+      Botan::SHA_160 sha;
+      Botan::byte hash[20] = { 0 };
+
+      sha.update(dat());
+      sha.final(hash);
+
+      std::string str_hash;
+      str_hash.append(reinterpret_cast<const char*>(hash), sizeof(hash));
+
+      ident = id(str_hash);
     }
   catch (Botan::Exception & e)
     {