lkml.org 
[lkml]   [2007]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] LZO de/compression support - take 3
On 5/24/07, Nitin Gupta <nitingupta910@gmail.com> wrote:
> [...]
> > > > diff --git a/lib/lzo1x/lzo1x_int.h b/lib/lzo1x/lzo1x_int.h
> > > > [...]
> > > > +/* Macros for 'safe' decompression */
> > > > +#ifdef LZO1X_DECOMPRESS_SAFE
> > > > +
> > > > +#define lzo1x_decompress lzo1x_decompress_safe
> > > > +#define TEST_IP (ip < ip_end)
> > > > +#define NEED_IP(x) \
> > > > + if ((size_t)(ip_end - ip) < (size_t)(x)) goto input_overrun
> > > > +#define NEED_OP(x) \
> > > > + if ((size_t)(op_end - op) < (size_t)(x)) goto output_overrun
> > > > +#define TEST_LB(m_pos) \
> > > > + if (m_pos < out || m_pos >= op) goto lookbehind_overrun
> > > > +#define HAVE_TEST_IP
> > > > +#define HAVE_ANY_OP
> > > > +
> > > > +#else /* !LZO1X_DECOMPRESS_SAFE */
> > > > +
> > > > +#define TEST_IP 1
> > > > +#define TEST_LB(x) ((void) 0)
> > > > +#define NEED_IP(x) ((void) 0)
> > > > +#define NEED_OP(x) ((void) 0)
> > > > +#undef HAVE_TEST_IP
> > > > +#undef HAVE_ANY_OP
> > > > +
> > > > +#endif /* LZO1X_DECOMPRESS_SAFE */
> > >
> > > ... ugh. Yes, extracting the common stuff between the _safe and _unsafe
> > > variants in a common low-level __lzo1x_decompress kind of function
> > > definitely looks doable. The low-level function could simply take an extra
> > > argument (say, set by the _safe and _unsafe wrappers) that tells it
> > > whether it is being called as safe or unsafe ... helps us get rid of the
> > > disruptions to all the Makefiles above and these #ifdef's ugliness ...
> >
> > I suspect it will probably damage performance unless the compiler is
> > very clever and I don't trust compilers that much...
>
> +1. I looked into Satyam suggestion as above but ...yes, we should not
> leave everything to compiler. And since all this was suggested just
> to do away with that symlink, I don't think this splitting work is
> worth the effort.

Not just the symlink ... we get rid of the changes in the two Makefiles,
the -D...SAFE stuff, some #ifdef's _and_ the macros listed above. Code
becomes even smaller and simpler. But yes, as I said, there'd be an
extra condition tested by the _unsafe variant (and *only* the _unsafe
variant, I must add) at the locations of these macros that was compiled
away previously. As for performance hit, I'd be interested in actually
measuring it first ... I definitely wouldn't mind trading off a couple of %
for a simpler and smaller patch.

But if you don't want even _that_ performance hit, you could also
simply duplicate the decompress() like Richard has done.

> For rigous testing, I have sent 'compress-test' module (with usage) to
> Bret Towe who has 64-bit machines available for testing.

This test module isn't so performance benchmarking friendly, good
enough for checking correctness of the algorithm implementation and
robustness of the code. But I think Richard's cryptoapi glue code patch
would work for your version too, as both your interfaces are the same,
so that'd be a better way to benchmark the relative performance of
your patch vs his. If you can test/benchmark both your versions and
produce results that beat his code, you've made it :-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-05-24 22:13    [W:0.067 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site