Re: [RFC] [-mm] Remove 'unsafe' LZO decompressor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Richard Purdie wrote:
On Thu, 2007-05-24 at 11:50 -0700, Andrew Morton wrote:
On Thu, 24 May 2007 18:15:17 +0100
Michael-Luke Jones <[email protected]> wrote:
		
Attached is a patch which may be desirable for -mm. It applies directly to 2.6.22-rc2-mm1.

The patch removes the 'unsafe' LZO decompression function, lowering the size of the minilzo.c file by nearly 500 out of an original 1727 lines. It also removes references to the 'unsafe' decompression function in the public LZO header and the EXPORT_SYMBOL_GPL declaration.
[...]
Comments / disagreement all welcome :)
This is obviously a highly desirable thing to do for a number of reasons. But have we quantified the performance difference?

Ok, I've done some tests:
1. Comparing the safe and unsafe functions

For my minilzo kernel patch, the safe version showed a 7.2% performance
hit. For Nitin's patch, it showed a 3.2% performance hit (but see
below).

Could be a lot worse and I don't object to the removal of the unsafe
version.

2. Comparing Nitin's code with my minilzo based kernel patch.

My kernel patch is about 2.25 times faster at decompression (16725Kb/ms
vs 7399Kb/ms) and fractionally faster at compression (1434kb/ms vs
1490kb/ms). As things stand the performance of Nitin's patch is
therefore unacceptable as that is a significant decompression
performance loss.

Please do _not_ rewrite the LZO implementation just for coding style principles.

The current miniLZO implementation is _extrememly_ well tested, pretty optimized and quite portable.

I agree that the implementation may look confusing, but you should be able to make it look much better by removing all the unused #defines and #ifdef code paths - LZO supports exotic things like 16-bit DOS and CRAY PVP memory models which obviously are not needed in the kernel and account for quite a number of abstractions (which are implemented through the preprocessor).

Finally the current version has been tested with a lot of compilers and contains accumulated knowledge about some hairy things - see http://gcc.gnu.org/PR25196 for an example, as well as some not-yet identified aliasing issue.

~Markus


These tests are on 32 bit Intel and in userspace but I've found them to
be a pretty good indicator of what happens in the real world and on
other architectures. For simplicity I made these tests with some existing code I had around
but its licence is such I can't share it, much to my frustration.

Cheers,

Richard



--
Markus Oberhumer, <[email protected]>, http://www.oberhumer.com/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux