Re: [RFC] LZO de/compression support - take 6

["Nitin Gupta" <nitingupta910@gmail.com>]

On 5/28/07, Daniel Hazelton <dhazelton@enter.net> wrote:
> On Monday 28 May 2007 10:40:31 Nitin Gupta wrote:
> > Hi,
> >
> > Attached is tester code used for testing.
> > (developed by Daniel Hazelton -- modified slightly to now use 'take 6'
> > version for 'TinyLZO')
> >
> > Cheers,
> > Nitin
> >
>
> <snip>
> I haven't tested with version 6, but after removing the LZO_CHECK_MPOS_NON_DET
> macro from the 'take 5' code and replacing the open-coded byte-for-byte
> copies with calls to memcpy:
>

I did memcpy() changes in some initial post (take '2', I think). That
caused some _correctness_ issue in de/compressor code -- Bret's test
could not succeed on ppc machine. After going back to byte-by-byte
copying, his tests were successful.

So, it's better not to include such changes now or test on all
supported archs if you really want to do so :)

> 10000 run averages:
> 'Tiny LZO':
>         Combined: 57.4691 usec
>         Compression: 39.8837 usec
>         Decompression: 17.5854 usec
> 'miniLZO':
>         Combined: 64.0484 usec
>         Compression: 46.0604 usec
>         Decompression: 17.988 usec
>
> which means:
> Overall TinyLZO is 10.2% faster
> TinyLZO compresses 13.4% faster
> TinyLZO decompresses 2.23% faster
>
> -Benchmark run a a Pentium-M 1.73GHz, 1GB Ram
> With the speed-up seen with just the removal of the LZO_CHECK_MPOS_NON_DET I
> wasn't sure that changing the open-coded copy to a call to memcpy() was going
> to have a big impact on the code, but it does appear to have has several
> percentage points of difference.

Yes, memcpy() changes have potential of giving significant perf gain
but I am not too sure if memcpy() will be good if we want to copy just
few bytes (which is the case at many times in de/compressor). Also, at
some places, memcpy() changes are not trival and have actually caused
correctness issues as mentioned above.

So, before this change, it will be good if it gets merged in mainline
and tested, at least for correctness, on all supported achs. All the
while, we will have a good feeling that there is still a good scope
for perf improvement :)


Cheers,
Nitin