[Clam-devel] [PATCH] std::vector operator[] and not calling frameSize()

[Roman Goj]

Hi!

Finally I managed to cook up something which should actually be useful
:) (contrary to the previous "processing time improvement" patch...
btw. is it not ok? it didn't get comitted... )

I learned to use valgrind (Pau, thanks for the tip, I would have had
doubts whether to use it or try anything else :) ) ... and it turns
out that right after fftw_execute, the biggest memory hog is the []
operator in std::vector and since _spectrum is a vector and it has to
be assigned literally millions of times in ConstantQTransform.cxx,
TonalAnalysis.cxx and FourierTransform.cxx I thought optimizing that
was the way to go.

The hack I did seems a bit dirty (at least to me)
double * spectrumArray = &_spectrum[0];
and then instead of calling _spectrum[i] I call spectrumArray[i]
and this turns out to be something like 5 times faster... I guess
iterators would have been a nicer way to do this, but I think they
would also be slower...

This site: (quite interesting, a nice find :) )
http://www.gotw.ca/gotw/050.htm
says this method is actually quite valid :) (should I dwelve deeper in
the site and maybe use the later tips also?)

Plus there were two loops which called frameSize() like this:
for(i=0; i<frameSize(); i++)
and it turns out the compiler isn't intelligent enough to optimize
this (I never cared about this, always thought gcc would take care of
this for me, I'm disappointed :( ) and calls frameSize everytime the
stopping condition is evaluated... so I put the result of frameSize()
in a variable and used that (a couple percent of improvement)

Conclusions:
I checked out the svn code today (would have sent the patch much
earlier but turns out there was something wrong with the way I was
building CLAM and I had some trouble with that :( ), compiled it and
issued:
./ChordExtractor -f .pool Debaser-CoffeeSmell.mp3

which resulted in:
Processing time: 585 seconds

then I recompiled it using fftw3 (why isn't this turned on by
default?), tried ChordExtractor again and got:
Processing time: 294 seconds

then I put the optimizations-vectors-into-arrays-frameSize-as-variable-not-function.patch
to work and got:
Processing time: 187 seconds

So the patch gives 36% improvement compared to the fftw3 version :)
(this doesn't take into account the improvement in TonalAnalysis.cxx,
which is also audible, but I didn't benchmark it that well)

Also doing and exhaustive search for a plan helps:
Processing time: 130 seconds
but, as I previously posted, this requires some additional
calculations (lots of them) beforehand, I'll try to catch you on #clam
tomorrow to ask about this...

So... I hope you accept these changes :) (And could anyone tell me how
it works on a better computer? On my duron 700 with the changes
TonalAnalysis in NetworkEditor works better then the cvs version on my
sempron 2,4... so I guess that's nice ;) )

Cheers!
Roman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: optimizations-vectors-into-arrays-frameSize-as-variable-not-function.patch
Type: text/x-diff
Size: 4447 bytes
Desc: not available
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20070614/3ab801e5/attachment-0004.patch>