Code review - Issue 12894043: code review 12894043: archive/zip: remove an allocation, speed up a testhttps://codereview.appspot.com/2013-08-14T02:32:34+00:00rietveld
Message from unknown
2013-08-13T21:29:35+00:00bradfitzurn:md5:5e5083316e7cff6b26bc3a04314446cf
Message from unknown
2013-08-13T21:29:38+00:00bradfitzurn:md5:a6aec385bef0b32c2680bf21f0064434
Message from unknown
2013-08-13T21:29:41+00:00bradfitzurn:md5:db0f04a2dae750e7d5105ecf1ee2a75a
Message from bradfitz@golang.org
2013-08-13T21:29:45+00:00bradfitzurn:md5:30309af9b9c3a18f73ef6454d6c0709a
Hello golang-dev@googlegroups.com,
I'd like you to review this change to
https://go.googlecode.com/hg/
Message from rsc@golang.org
2013-08-13T21:47:11+00:00rscurn:md5:736a97634f6bcbfce3abae5edd0a3683
LGTM
Message from unknown
2013-08-13T21:47:59+00:00bradfitzurn:md5:404096727a3e968cd63e77db8561f88e
Message from bradfitz@golang.org
2013-08-13T21:48:11+00:00bradfitzurn:md5:bdce7d14bd8d8ca8d45e8a719276dddc
*** Submitted as https://code.google.com/p/go/source/detail?r=2bc5fc177d58 ***
archive/zip: remove an allocation, speed up a test
Update Issue 6138
TestOver65kFiles spends all its time garbage collecting.
Removing the 1.4 MB of allocations per each of the 65k
files brings this from 34 seconds to 0.23 seconds.
R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/12894043
Message from adg@google.com
2013-08-14T02:32:34+00:00adg1urn:md5:99e92dbc87e24ee779b22c1671eec2f3
LGTM
I knew this was inefficient but I didn't realise how bad. Thanks.
On 14 Aug 2013 07:29, <bradfitz@golang.org> wrote:
> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> archive/zip: remove an allocation, speed up a test
>
> Update Issue 6138
>
> TestOver65kFiles spends all its time garbage collecting.
> Removing the 1.4 MB of allocations per each of the 65k
> files brings this from 34 seconds to 0.23 seconds.
>
> Please review this at https://codereview.appspot.**com/12894043/<https://codereview.appspot.com/12894043/>
>
> Affected files:
> M src/pkg/archive/zip/reader.go
> M src/pkg/archive/zip/zip_test.**go
>
>
> Index: src/pkg/archive/zip/reader.go
> ==============================**==============================**=======
> --- a/src/pkg/archive/zip/reader.**go
> +++ b/src/pkg/archive/zip/reader.**go
> @@ -179,9 +179,8 @@
> // findBodyOffset does the minimum work to verify the file has a header
> // and returns the file body offset.
> func (f *File) findBodyOffset() (int64, error) {
> - r := io.NewSectionReader(f.zipr, f.headerOffset,
> f.zipsize-f.headerOffset)
> var buf [fileHeaderLen]byte
> - if _, err := io.ReadFull(r, buf[:]); err != nil {
> + if _, err := f.zipr.ReadAt(buf[:], f.headerOffset); err != nil {
> return 0, err
> }
> b := readBuf(buf[:])
> Index: src/pkg/archive/zip/zip_test.**go
> ==============================**==============================**=======
> --- a/src/pkg/archive/zip/zip_**test.go
> +++ b/src/pkg/archive/zip/zip_**test.go
> @@ -17,14 +17,14 @@
> )
>
> func TestOver65kFiles(t *testing.T) {
> - if testing.Short() {
> - t.Skip("slow test; skipping")
> - }
> buf := new(bytes.Buffer)
> w := NewWriter(buf)
> const nFiles = (1 << 16) + 42
> for i := 0; i < nFiles; i++ {
> - _, err := w.Create(fmt.Sprintf("%d.dat", i))
> + _, err := w.CreateHeader(&FileHeader{
> + Name: fmt.Sprintf("%d.dat", i),
> + Method: Store, // avoid Issue 6136 and Issue 6138
> + })
> if err != nil {
> t.Fatalf("creating file %d: %v", i, err)
> }
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegroups.com>
> .
> For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/opt_out>
> .
>
>
>