code review 12894043: archive/zip: remove an allocation, speed up a test

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.

[bradfitz, 2013-08-13 21:29:45]

Hello golang-dev@googlegroups.com,

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

[rsc, 2013-08-13 21:47:11]

LGTM

[bradfitz, 2013-08-13 21:48:11]

*** 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

[adg1, 2013-08-14 02:32:34]

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/128...
>
> 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@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>