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

cmd/compile: copy causes escape #15730

Closed
aclements opened this issue May 18, 2016 · 5 comments
Closed

cmd/compile: copy causes escape #15730

aclements opened this issue May 18, 2016 · 5 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@aclements
Copy link
Member

  1. What version of Go are you using (go version)?
    go version devel +0a9595f Tue May 17 18:46:03 2016 -0400 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
    GOARCH="amd64"
    GOBIN=""
    GOEXE=""
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"
    GOOS="linux"
    GOPATH="/home/austin/r/go"
    GORACE=""
    GOROOT="/home/austin/go.dev"
    GOTOOLDIR="/home/austin/go.dev/pkg/tool/linux_amd64"
    CC="gcc"
    GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build366275436=/tmp/go-build -gno-record-gcc-switches"
    CXX="g++"
    CGO_ENABLED="1"
  3. What did you do?
package main

var sink [100]byte

func test(args ...interface{}) {
    for _, arg := range args {
        switch arg := arg.(type) {
        case string:
            // copy causes args to escape and causes the
            // convT2E in main to allocate.
            copy(sink[:], arg)

            // Manually copying means args does not escape
            // and the convT2E in main does not allocate.
            //for i := 0; i < len(arg); i++ {
            //  sink[i] = arg[i]
            //}
        }
    }
}

func main() {
    test("abc")
}
  1. What did you expect to see?
    I expected "abc" in main to not escape and the conversion to interface{} to not allocate.
  2. What did you see instead?
    If I use copy in test, escape analysis determines that arg's content escapes, which causes the args slice to escape, which causes the convT2E in main to allocate. However, if I do the copy "manually" by looping over the string, args does not escape, and the convT2E happens on the stack.

/cc @dr2chase @randall77

@quentinmit quentinmit added this to the Go1.8 milestone Jun 17, 2016
@quentinmit
Copy link
Contributor

Assigning to 1.8 since this seems inefficient but not broken. Please reassign if you disagree.

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@rsc rsc modified the milestones: Go1.9Early, Go1.8 Oct 21, 2016
@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.9Early May 3, 2017
@bradfitz bradfitz added early-in-cycle A change that should be done early in the 3 month dev cycle. and removed early-in-cycle A change that should be done early in the 3 month dev cycle. labels Jun 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.10 Jun 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 28, 2017
@xiaost
Copy link

xiaost commented Apr 17, 2018

Any progress?

I'm try to fix this issue by this tricky code.
It works on Go1.10, but go vet warn me.

@josharian
Copy link
Contributor

cc @cherrymui

@cherrymui
Copy link
Member

The loop version doesn't escape because the assignment is on byte, a non-pointer type. The copy version is modeled with data flow of type "dereference-of-string", which was (conservatively) also string, which contains pointer, which causes escape.

@gopherbot
Copy link

Change https://golang.org/cl/107597 mentions this issue: cmd/compile: in escape analysis, use element type for OIND of slice

@golang golang locked and limited conversation to collaborators Jul 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

8 participants