mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Nitin Gupta" <nitingupta910@gmail.com>
To: "Markus F.X.J. Oberhumer" <markus@oberhumer.com>
Cc: "Richard Purdie" <richard@openedhand.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Michael-Luke Jones" <mlj28@cam.ac.uk>,
	lkml <linux-kernel@vger.kernel.org>,
	"Satyam Sharma" <satyam.sharma@gmail.com>
Subject: Re: [RFC] [-mm] Remove 'unsafe' LZO decompressor
Date: Fri, 25 May 2007 12:05:26 +0530	[thread overview]
Message-ID: <4cefeab80705242335s399b20aek61cebc31260ad01@mail.gmail.com> (raw)
In-Reply-To: <4656307F.6010204@oberhumer.com>

Hi Markus,

On 5/25/07, Markus F.X.J. Oberhumer <markus@oberhumer.com> wrote:
> 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

I did not rewrite any part of your code except replacing COPY4() macro
and some open-coded byte-by-byte copying with memcpy(). But this has
resulted in very significant perf. loss (as suggested by results from
Richard's tests) - so will rollback these changes.

Additionally following was done to _greatly_ reduce no. of LOC I had
to retain. These should not affect code correctness and performance:
- Used standard/kernel defined data types equivalent of lzo_* types.
This resulted in removal of huge chunks of #ifdefs:
    lzo_byptep -> unsigned char *
    lzo_uint -> size_t
    lzo_xint -> size_t
    lzo_uintptr_t -> unsigned long
    lzo_uint32p -> uint32_t *
- Removed everything #ifdefed under COPY_DICT  -- from minilzo code I
see that this is not #defined for LZO1X (safe/unsafe) (though I could
not understand meaning behind COPY_DICT).
- Removed everthing #ifdef'ed for LZO1Y, LZO1Z, other variants.


Thanks,
Nitin

  reply	other threads:[~2007-05-25  6:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-24 17:15 Michael-Luke Jones
2007-05-24 18:50 ` Andrew Morton
2007-05-24 19:13   ` Michael-Luke Jones
2007-05-24 22:26   ` Richard Purdie
2007-05-25  0:40     ` Markus F.X.J. Oberhumer
2007-05-25  6:35       ` Nitin Gupta [this message]
2007-05-25  6:10     ` Nitin Gupta
2007-05-29 19:43 ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4cefeab80705242335s399b20aek61cebc31260ad01@mail.gmail.com \
    --to=nitingupta910@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus@oberhumer.com \
    --cc=mlj28@cam.ac.uk \
    --cc=richard@openedhand.com \
    --cc=satyam.sharma@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®