[MERGE] tuned_gzip.bytes_to_gzip

[Robert Collins]

On Mon, 2007-09-17 at 17:34 +1000, Ian Clatworthy wrote:


> Some other comments below.
> 
> > +        bytes = (''.join(chain(
> >              ["version %s %d %s\n" % (version_id,
> >                                       len(lines),
> >                                       digest)],
> >              lines,
> > -            ["end %s\n" % version_id]))
> > -        data_file.close()
> > -        length= sio.tell()
> > -
> > -        sio.seek(0)
> > -        return length, sio
> > +            ["end %s\n" % version_id])))
> 
> Is the enclosing () needed around bytes?

mm, strictly no, but it read more clearly to me.

> > +def bytes_to_gzip(bytes, factory=zlib.compressobj,
> > +    level=zlib.Z_DEFAULT_COMPRESSION, method=zlib.DEFLATED,
> > +    width=-zlib.MAX_WBITS, mem=zlib.DEF_MEM_LEVEL,
> > +    crc32=zlib.crc32):
> 
> A comment or two separate to your patch ...
> 
> 1. John might have some figures on various compression levels, their
> performance and space tradeoffs. IIUIC, Z_DEFAULT_COMPRESSION is -1
> which maps to 6 on a scale of 0-9, 9 being the slowest and most
> compression. Less compression is necessarily faster (overall) of course.

Actually its a U curve. No compress = lots of disk. High compress = lots
of CPU.

-1 is 6 *currently* buts its defined as being basically the sweet spot
for most users most of the time, so using Z_DEFAULT_COMPRESSION is
almost invariably a good idea.

> 2. crc=zlib.alder32 might be slightly quicker as well. Changing this
> looks more complex that just changing the compression level though
> because I'm guessing that's a different file format (unless a flag is
> used to indicate the type of crc?). Our decompress code certainly looks
> hard-coded to crc32 (and fair enough given it's goal of being optimised).

The crc needs to stay the same, its wire level compatibility, and
adler32 is not byte order safe IIRC (we had a problem with dirstate with
those two in the past).

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070917/a8a58db8/attachment.pgp