-
Notifications
You must be signed in to change notification settings - Fork 649
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
bucket: allow to allocate key on stack in Put() #550
Conversation
Cool! Can we have a short comment in code that explains why |
338fe85
to
8cf4dc2
Compare
Fixed, please, take a look. |
8cf4dc2
to
340852a
Compare
I dived a bit deeper and it seems fixing this in compiler is not that easy (it is not run on SSA form yet, so all |
As per `go build -gcflags -m ./... 2>&1`: Old behaviour: ``` ./bucket.go:148:31: leaking param: key ./bucket.go:192:42: leaking param: key ./bucket.go:271:22: leaking param: key ``` Now: ``` ./bucket.go:148:31: key does not escape ./bucket.go:192:42: key does not escape ./bucket.go:271:22: key does not escape ``` Signed-off-by: Evgenii Stratonikov <fyfyrchik@runbox.com>
340852a
to
71a59ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you! @fyfyrchik
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
- [bucket: allow to allocate key on stack in Put()](etcd-io/bbolt#550) - [bucket.Put: copy key before seek](etcd-io/bbolt#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io/bbolt#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com> Signed-off-by: samuelbartels20 <bartelssamuel20@gmail.com>
While doing some bbolt optimizations for my usecase I have noticed key for
Bucket.Get()
andBucket.Delete()
can be allocated on stack, but when I addBucket.Put()
it cannot. While in reality it could, it seems to be hard for the go compiler.As per
go build -gcflags -m ./...
:Old behaviour:
Now:
I have performed smoke tests on the benchmark from here #532, and got the expected results: 1 less allocation.