code review 5554066: compress/flate: make lazy matching work.

compress/flate: make lazy matching work.

[imkrasin, 2012-01-19 18:10:31]

Russ,

the fix is ready for the review. Before committing, I would like to add more
DeflateInflate tests to ensure that these changes won't produce corrupted files
on compression level > 3.

[rsc, 2012-01-19 18:31:52]

Nice, thanks.

Can we define

const neverSkipHashing = math.MaxInt32

or something like that?

[imkrasin, 2012-01-20 02:16:52]

Done.

I have added the book (public domain) as a comprehensive deflate-inflate ascii
test, but I'm not sure it's worth the slowdown (+1.2s)

I have ran deflate-inflate correctness check on 10k books from Project Gutenberg
in both forms: .txt and .zip and with all compression levels. No issues, which
is probably a good sign.

[imkrasin, 2012-01-20 07:16:40]

Russ, 

please, take a look.

[rsc, 2012-01-20 17:48:34]

Should the compressed form of the test data
be checked against a maximum size for each
compression level?

Russ

[imkrasin, 2012-01-20 22:32:11]

Done.

[rsc, 2012-01-20 22:38:52]

looks good but please address the limit count problem.
also please run hg mail 5554066 so it goes to golang-dev.

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
File src/pkg/compress/flate/deflate_test.go (right):

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
src/pkg/compress/flate/deflate_test.go:286: limit    []int
Please make this limit [10]int

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
src/pkg/compress/flate/deflate_test.go:293: []int{10013, 5065, 5096, 5115, 5093,
5079, 5079, 5079, 5079, 5079},
Please make this [...]int.
Then if there are not 10 ints, you will get a compiler error.

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
src/pkg/compress/flate/deflate_test.go:298: []int{416188, 191483, 185232,
179560, 175233, 171263, 169908, 169758, 169712, 169712},
Same.

I ask because there are 10 ints above and 9 ints here.

[Ivan Krasin, 2012-01-20 22:44:44]

Hello rsc@golang.org, imkrasin@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[imkrasin, 2012-01-20 22:44:51]

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
File src/pkg/compress/flate/deflate_test.go (right):

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
src/pkg/compress/flate/deflate_test.go:286: limit    []int
On 2012/01/20 22:38:52, rsc wrote:
> Please make this limit [10]int

Done.

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
src/pkg/compress/flate/deflate_test.go:293: []int{10013, 5065, 5096, 5115, 5093,
5079, 5079, 5079, 5079, 5079},
On 2012/01/20 22:38:52, rsc wrote:
> Please make this [...]int.
> Then if there are not 10 ints, you will get a compiler error.

Done.

http://codereview.appspot.com/5554066/diff/1016/src/pkg/compress/flate/deflat...
src/pkg/compress/flate/deflate_test.go:298: []int{416188, 191483, 185232,
179560, 175233, 171263, 169908, 169758, 169712, 169712},
On 2012/01/20 22:38:52, rsc wrote:
> Same.
> 
> I ask because there are 10 ints above and 9 ints here.
It's actually 10 here and 10 there, but I see your point.

[imkrasin, 2012-01-20 22:59:50]

Russ,

I'm not a committer. Could you please commit this CL for me?

Thanks!

[rsc, 2012-01-20 23:09:21]

LGTM

Will submit a bit later.

[rsc, 2012-01-21 04:35:23]

*** Submitted as fe4589a9405c ***

compress/flate: make lazy matching work.

R=rsc, imkrasin
CC=golang-dev
http://codereview.appspot.com/5554066

Committer: Russ Cox <rsc@golang.org>