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

context: documentation could suggest how to use Context key types efficiently #17826

Closed
nizsheanez opened this issue Nov 7, 2016 · 5 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@nizsheanez
Copy link

#17293 - defined that usage of basic types is terrible and proposed to use private type type key int.
But it cause allocation on each usage of context.
We can avoid allocation, by usage type key interface{}
Here is test:

package test

import (
	"context"
	"testing"
)

type key interface{}
var keyInterface key = 0

type keyIntType int
var keyInt keyIntType = 0

type List struct{}

func BenchmarkInterfaceKey(b *testing.B) {
	ctx := context.Background()
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			ctx.Value(keyInterface)
		}
	})
}

func BenchmarkIntKey(b *testing.B) {
	ctx := context.Background()
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			ctx.Value(keyInt)
		}
	})
}

Result:

BenchmarkIntKey-8         	500000000	        10.2 ns/op	       8 B/op	       1 allocs/op
BenchmarkInterfaceKey-8   	5000000000	         1.36 ns/op	       0 B/op	       0 allocs/op

Does it make sense to use type key interface{} in docs?

@bradfitz
Copy link
Contributor

bradfitz commented Nov 7, 2016

No need for that. You can just do:

var FooKey interface{} = foo

And the allocation (if any) will only be done once at program start-up, and then you can use FooKey as a key allocation-free whenever.

Or make your key be an empty struct type:

type MyKey struct{}

And then just:

     context.WithValue(ctx, MyKey{}, val)

... because putting MyKey{} (an empty struct) into an interface also doesn't allocate.

@bradfitz bradfitz added this to the Go1.8Maybe milestone Nov 7, 2016
@bradfitz bradfitz changed the title context: documentation should suggest usage of interface{} for private type declaration context: documentation could suggest how to use Context key types efficiently Nov 7, 2016
@nizsheanez
Copy link
Author

nizsheanez commented Nov 8, 2016

I found that usage of type key interface{} cause collisions:

type structType1 struct{}
type structType2 struct{}
var key1 structType1
var key2 structType2

type interfaceType1 interface{}
type interfaceType2 interface{}
var key3 interfaceType1 = 1
var key4 interfaceType2 = 1

func main() {
    ctx := context.Background()

    ctx = context.WithValue(ctx, key1, "Yes!")
    fmt.Printf("%s\n", ctx.Value(key2))

    ctx = context.WithValue(ctx, key3, "Sure!")
    fmt.Printf("%s\n", ctx.Value(key4))
}

// Result:
// StructKey: %!s(<nil>)
// InterfaceKey: Sure!

Yes, usage of interface{} is a bad idea, but struct{} look good.
For me looks valuable to:
Recommend don't use type key interface{}
Recommend to use empty struct type:

type keyType struct{}
...
context.WithValue(ctx, keyType{}, val)

or with global variable (looks more natural)

type keyType struct{}
type key keyType
...
context.WithValue(ctx, key, val)

Also now docs have `The key type is unexported to prevent collisions with context keys defined in
// other packages.` -  with `type key struct{}` it can be exported, different types don't cause collisions even if they in same package. 



@bradfitz
Copy link
Contributor

bradfitz commented Nov 8, 2016

I think you're confused about how interfaces and types in general work in Go.

Your key3 and key4 are identical.

See https://play.golang.org/p/fyBgLguaOb and http://research.swtch.com/interfaces

@nizsheanez
Copy link
Author

Actually, I and people in my team know, but it coming to head only when facing issue :-)
Same when people think need to use cheap data type here - they use int instead of struct{} (they just don't remember about existing of struct{} in this time, even they know that it's cheaper)

Anyway, benchmark int vs empty struct has only 2 time difference in speed:

BenchmarkStructKey-8    300000000            4.45 ns/op        0 B/op          0 allocs/op
BenchmarkIntKey-8       200000000            8.24 ns/op        8 B/op          1 allocs/op

Even speed is almost same, I think collision-free is valuable in type key struct{} usage

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 8, 2016
@gopherbot
Copy link

CL https://golang.org/cl/33152 mentions this issue.

@golang golang locked and limited conversation to collaborators Nov 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants