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: Optimize strings.Join with finite number of elements #16157

Closed
dlsniper opened this issue Jun 22, 2016 · 4 comments
Closed

cmd/compile: Optimize strings.Join with finite number of elements #16157

dlsniper opened this issue Jun 22, 2016 · 4 comments
Labels
FrozenDueToAge Performance
Milestone
Go1.8Maybe

Comments

@dlsniper
Copy link
Contributor

dlsniper commented Jun 22, 2016 •
edited

  1. What version of Go are you using (go version)?
    go version go1.7beta2 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"

Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz

Given the following example: https://play.golang.org/p/a_ow6e7II7

I would expect the compiler to transform the calls to strings.Join with a finite number of elements into simple string concatenations. The reason is that these calls might be used on hot-paths for code, for example: aws/aws-sdk-go#729

The results might be small, see below for my machine, but any speed improvement is a good one imho

go test -bench=.
BenchmarkStringsJoin-8          10000000               140 ns/op
BenchmarkStringsConcat-8        20000000                92.6 ns/op
PASS
ok      some/package        3.505s


go test -bench=. -benchmem
BenchmarkStringsJoin-8          10000000               136 ns/op             128 B/op          2 allocs/op
BenchmarkStringsConcat-8        20000000                96.6 ns/op            64 B/op          1 allocs/op
PASS
ok      some/package        3.565s

Thank you.
@randall77
Copy link
Contributor

Could be done. The simple fix is to make strings.Join work similarly to runtime.concatstrings, in particular to use rawstring instead of doing the concat in a byte slice and then converting to a string.

A bigger change would be to do semantic inlining of strings.Join in the compiler.

@randall77 randall77 added this to the Go1.8Maybe milestone Jun 22, 2016
@bradfitz
Copy link
Contributor

A bigger change would be to do semantic inlining of strings.Join in the compiler.

That seems too big. Most of the standard library should not be special. Maybe parts of sync, runtime, and time can be recognized by the compiler, but not high-level packages likes "strings".

@bradfitz
Copy link
Contributor

FWIW, 3ddc9ad was already submitted to optimize the common cases of strings.Join.

@randall77, I'm of the opinion that this bug should be closed and the compiler shouldn't special case things in the standard library, at least not in regular (unprivileged) packages like "strings".

@randall77
Copy link
Contributor

Sure, I'll agree with that. Note that #6714 will fix this issue as a side effect.

@golang golang locked and limited conversation to collaborators Aug 20, 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
Go1.8Maybe
Development

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