Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compress/flate: Write() causes large and unshrinkable stack growth #18625

Closed
y3llowcake opened this issue Jan 12, 2017 · 10 comments
Closed

compress/flate: Write() causes large and unshrinkable stack growth #18625

y3llowcake opened this issue Jan 12, 2017 · 10 comments

Comments

@y3llowcake
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.7.1 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN="/home/cy/go/go-1.7.1/gopath/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/cy/go/go-1.7.1/gopath"
GORACE=""
GOROOT="/home/cy/go/go-1.7.1"
GOTOOLDIR="/home/cy/go/go-1.7.1/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build525622235=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

package main

import (
        "compress/flate"
        "fmt"
        "io/ioutil"
        "runtime"
        "runtime/debug"
)

const limit = 1024 * 16

func printss() {
        m := runtime.MemStats{}
        runtime.ReadMemStats(&m)
        fmt.Printf("inuse: %d sys: %d\n", m.StackInuse, m.StackSys)
}

func doflate() {
        z := make([]byte, limit*1000, limit*1000)
        fl, _ := flate.NewWriter(ioutil.Discard, 3)
        printss()
        fl.Write(z) // boom.
}

func main() {
        fmt.Println("hi")
        debug.SetMaxStack(limit) // comment out for more detail.
        doflate()                // boom.
        printss()
        runtime.GC()
        printss()
}

What did you expect to see?

Passing a large input to flate.Write() would not cause the stack to grow beyond the limit provided.

What did you see instead?

The stack grows very large.

Also, if I comment out this line:
debug.SetMaxStack(limit)

I get the following output:

hi
inuse: 393216 sys: 393216
inuse: 1474560 sys: 1474560
inuse: 950272 sys: 950272

Which suggests the runtime managed to shrink the stack a bit, but not nearly close enough to the original size.

Now imagine you have written a server that has lots of long running goroutines that occasionally compress a large blob. After introducing this code path, our stack usage increased ~10x.

@dsnet dsnet self-assigned this Jan 12, 2017
@dsnet dsnet added this to the Go1.9 milestone Jan 12, 2017
@dsnet
Copy link
Member

dsnet commented Jan 12, 2017

Wow. Fun bug. The problem is that we try to iterate over an array, which copies it to the stack. The arrays in question are hashHead and hashPrev, which we iterate over in these loops.

@gopherbot
Copy link

CL https://golang.org/cl/35122 mentions this issue.

@dsnet
Copy link
Member

dsnet commented Jan 12, 2017

\cc @klauspost, I believe your library probably has the same bug.

@klauspost
Copy link
Contributor

@dsnet - thanks for notifying me.

Alternatively we could allocated them. That would also make "Best Speed" faster, since it does not need these two buffers.

@cherrymui
Copy link
Member

Having not looked at the source code, I wonder why the compiler makes such copy.

@dsnet
Copy link
Member

dsnet commented Jan 12, 2017

Ironically, @dgryski had a twitter post about this, recently:

Today's #golang gotcha: the two-value range over an array does a copy. Avoid by ranging over the pointer instead.
https://play.golang.org/p/4b181zkB1O

@cherrymui
Copy link
Member

@dsnet, thanks!
This is required by spec:

The range expression is evaluated once before beginning the loop...

So this comes from the difference between evaluating an array vs. evaluating a slice... Still, this behavior is somewhat surprising.

@dsnet dsnet modified the milestones: Go1.8, Go1.9 Jan 12, 2017
@valyala
Copy link
Contributor

valyala commented Jan 13, 2017

For those who wants small stacks with compress/* on go versions up to 1.7 - try using https://godoc.org/github.com/valyala/fasthttp/stackless . This is an ugly workaround we use in our production servers :) It just creates GOMAXPROCS goroutines dedicated for calling Write method on compress/* writers and passes Write calls to these goroutines via channels.

@minux
Copy link
Member

minux commented Jan 13, 2017 via email

@klauspost
Copy link
Contributor

Using "BestSpeed" (level 1) should AFAICT solve the issue for Go 1.7.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants