[PATCH 2/2] (g)unzip: Optimize inflate_codes()

[Joakim Tjernlund]

Rob Landley <rob at landley.net> wrote on 2010/02/10 03:21:27:
>
> On Monday 08 February 2010 16:04:40 Joakim Tjernlund wrote:
> > > Joakim Tjernlund/Transmode wrote on 2010/02/08 22:52:11:
> > > > Rob Landley <rob at landley.net> wrote on 2010/02/08 21:26:44:
> > > > > On Monday 08 February 2010 11:55:16 Joakim Tjernlund wrote:
> > > > > > Ported the recent optimization from the Linux kernel.
> > > > > > This will not perform as god as the kernel version as the
> > > > > > code structure in busybox is different and I had to adopt
> > > > > > the optimization to it.
> > > > > >
> > > > > > This has seen very little testing and is a RFC only at this point.
> > > > > > The inflate speed increase in the kernel was 12-15% on ppc.
> > > > > >
> > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> > > > >
> > > > > That looks like a fairly significant size increase.  Is it big enough
> > > > > to justify a config option?  (This is why "make bloatcheck"
> > > > > exists...)
> > > >
> > > > function                                             old     new
> > > > delta inflate_codes                                        624     819
> > > >   +195 .rodata                                           131741  131755
> > > >     +14
> > > > -----------------------------------------------------------------------
> > > >------- (add/remove: 0/0 grow/shrink: 2/0 up/down: 209/0)
> > > > Total: 209 bytes text    data     bss     dec     hex filename
> > > >  705967    2137    9072  717176   af178 busybox_old
> > > >  706176    2137    9072  717385   af249 busybox_unstripped
> > >
> > > Left some debug in, the correct bloatcheck is:
> > > function                                             old     new   delta
> > > inflate_codes                                        624     791    +167
> > > -------------------------------------------------------------------------
> > >----- (add/remove: 0/0 grow/shrink: 1/0 up/down: 167/0)             Total:
> > > 167 bytes text      data       bss       dec       hex   filename
> > >  705967      2137      9072    717176     af178   busybox_old
> > >  706134      2137      9072    717343     af21f   busybox_unstripped
> >
> > And yet some:
> > function                                             old     new   delta
> > inflate_codes                                        624     728    +104
> > ---------------------------------------------------------------------------
> >--- (add/remove: 0/0 grow/shrink: 1/0 up/down: 104/0)             Total: 104
> > bytes text      data       bss       dec       hex   filename
> >  705967      2137      9072    717176     af178   busybox_old
> >  706071      2137      9072    717280     af1e0   busybox_unstripped
> >
> > I don't think this should be a config option considering the increased risk
> > of bugs with two variants. Inflate bugs can be pretty serious to your data.
>
> For ~100 bytes I agree, I just wanted to make sure we'd evaluated it.  It
> seems pretty reasonable, and 104 bytes is better than 209. :)
>
> The 12-15% speedup mentioned was on powerpc, what platform was the size
> increase measured on?  (Alas, the one thing QEMU is _not_ good for is

The size mentioned here was on x86 and the perf test was on mpc83xx.

> performance testing.  The performance characteristics of the emulator and the
> real hardware are different enough that micro-optimizing for one can actually
> slow the other down...)
>
> By the way, does our gzip code have an "upstream" we follow?  I.E. would it be

BB seems to follow upstream gzip but the kernel follows zlib. gzip seems to be based
on an old zlib and recent zlib ha changed a lot. I suspect zlib is much faster
than gzip.

> easy to fairly closely mirror the kernel's inflate/deflate code so we can track
> any future changes more easily?  For example, the kernel version diverged
> significantly from both zlib and gzip, and the kernel version is gplv2 (only).
> We want to make sure _not_ to blindly copy changes from gnu gzip because gzip
> 1.4 is GPLv3.

Ah, gzip is now GPLv3. Good to know.