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

replace context.WithValue string key to typed struct{} key #11634

Closed
lysu opened this issue Aug 6, 2019 · 1 comment · Fixed by #11675
Closed

replace context.WithValue string key to typed struct{} key #11634

lysu opened this issue Aug 6, 2019 · 1 comment · Fixed by #11675
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/performance

Comments

@lysu
Copy link
Contributor

lysu commented Aug 6, 2019

Current TiDB many places uses a string as the context key which leads to allocations.

for example:

ctx = context.WithValue(ctx, txnStartKey, req.StartTs)

ctx = context.WithValue(ctx, execdetails.CommitDetailCtxKey, &commitDetail)

https://golang.org/pkg/context/#WithValue

To avoid allocating when assigning to an interface{}, context keys often have concrete type struct{}. Alternatively, exported context key variables' static type should be a pointer or interface.

https://www.dropbox.com/s/9u74uvpq8vowwdo/Screenshot%202019-07-30%2012.39.20.png?dl=0

instead, we'd better using type xKey struct{} just like opentracing does https://github.com/opentracing/opentracing-go/blob/master/gocontext.go#L12 to reduce alloc

@lysu lysu added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/performance labels Aug 6, 2019
@lysu lysu changed the title Replace context.WithValue string key to struct{} key replace context.WithValue string key to struct{} key Aug 6, 2019
@lysu lysu changed the title replace context.WithValue string key to struct{} key replace context.WithValue string key to typed struct{} key Aug 6, 2019
@lauhg
Copy link
Contributor

lauhg commented Aug 7, 2019

I can work on this if nobody else is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants