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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: converting []byte to string could re-use non-escaping memory #16408

Closed
bradfitz opened this issue Jul 18, 2016 · 4 comments
Closed

cmd/compile: converting []byte to string could re-use non-escaping memory #16408

bradfitz opened this issue Jul 18, 2016 · 4 comments
Labels
FrozenDueToAge Performance

Comments

@bradfitz
Copy link
Contributor

In a program like:

package alloc_test

import "testing"

var s = "some string to be doubled by foo"

func foo() string {
        buf := make([]byte, len(s)*2)
        copy(buf, s)
        copy(buf[:len(s)], s)
        return string(buf)
}

func BenchmarkGarbage(b *testing.B) {
        b.ReportAllocs()
        for i := 0; i < b.N; i++ {
                foo()
        }
}

The string(buf) conversion allocates memory, even though the buf variable should not escape, and is only used by string(buf) at the exit of the function.

This results in two allocations per call to foo:

BenchmarkGarbage-2   20000000     108 ns/op     128 B/op     2 allocs/op

More practically, this affects things like filepath.Join or path.Join which uses strings.Join (which looks like foo above)

I thought we had this bug open already but I can't find it.

/cc @randall77 @josharian
@randall77
Copy link
Contributor

Reusing the storage of the byte slice in string([]byte) should be doable.
The escape analysis pass already provides the escape info we need, I believe.
The additional check would be to make sure that the []byte is not modified after the string cast by the function being compiled. I'm not sure what mechanism we'd need for that. Maybe just an imminent return after the cast would catch most cases we want.

@bradfitz
Copy link
Contributor Author

Maybe just an imminent return after the cast would catch most cases we want.

I suspect so.

@nussjustin
Copy link
Contributor

This is #6714 :-)

@bradfitz
Copy link
Contributor Author

@nuss-justin, indeed! Thanks. Closing this as a dup.

@golang golang locked and limited conversation to collaborators Jul 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Performance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bradfitz @nussjustin @randall77 @gopherbot