context: remove allocation discussion from WithValue documentation

[evanj]

This is a proposed package documentation change. I'm happy to submit a code change with this update, if it makes sense. I took the liberty of abbreviating the questions in the template.

What version of Go are you using (go version)?

Documentation in the latest source of context.go (commit d6ffc1d8394d6f6420bb92d79d320da88720fbe0)

What does the current documentation say

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."

Current documentation: https://tip.golang.org/pkg/context/#WithValue Code: https://github.com/golang/go/blob/d6ffc1d8394d6f6420bb92d79d320da88720fbe0/src/context/context.go#L476-L479

What should it say

Those two sentences should be removed. With Go >= 1.9, it no longer matters. To verify, I ran the following test with different versions of Go on a VM with a command like:

docker run --workdir=/wtf/test -v $HOME:/wtf -ti --rm golang:1.12 go test -bench=. .

I tested each release from 1.12 through to 1.8. With version 1.8, this mattered a lot. It no longer does. From the output below, you can see that using an int key is slower, but does not allocate. The other choices (interface{}, pointer, custom string), all appear to be equivalent.

I think it would simplify the package documentation to omit this.

This was previously changed after the discussion in https://github.com/golang/go/issues/17826 . My test is based on that one.

Go 1.12

goos: linux
goarch: amd64
BenchmarkInterfaceKey-2      	1000000000	         2.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkIntKey-2            	300000000	         4.56 ns/op	       0 B/op	       0 allocs/op
BenchmarkStringKey-2         	1000000000	         2.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkCustomStringKey-2   	1000000000	         2.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkEmptyStructKey-2    	1000000000	         2.63 ns/op	       0 B/op	       0 allocs/op
BenchmarkPtrKey-2            	1000000000	         2.63 ns/op	       0 B/op	       0 allocs/op

Go 1.9

goos: linux
goarch: amd64
BenchmarkInterfaceKey-2      	1000000000	         2.66 ns/op	       0 B/op	       0 allocs/op
BenchmarkIntKey-2            	300000000	         5.28 ns/op	       0 B/op	       0 allocs/op
BenchmarkStringKey-2         	1000000000	         2.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkCustomStringKey-2   	1000000000	         2.63 ns/op	       0 B/op	       0 allocs/op
BenchmarkEmptyStructKey-2    	1000000000	         2.66 ns/op	       0 B/op	       0 allocs/op
BenchmarkPtrKey-2            	1000000000	         2.67 ns/op	       0 B/op	       0 allocs/op

Go 1.8

BenchmarkInterfaceKey-2      	300000000	         4.37 ns/op	       0 B/op	       0 allocs/op
BenchmarkIntKey-2            	50000000	        30.6 ns/op	       8 B/op	       1 allocs/op
BenchmarkStringKey-2         	30000000	        47.6 ns/op	      16 B/op	       1 allocs/op
BenchmarkCustomStringKey-2   	30000000	        47.6 ns/op	      16 B/op	       1 allocs/op
BenchmarkEmptyStructKey-2    	100000000	        15.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkPtrKey-2            	300000000	         4.57 ns/op	       0 B/op	       0 allocs/op

Test code

package test

import (
	"context"
	"testing"
)

type key interface{}

var keyInterface key = 0

type keyIntType int

var keyInt keyIntType = 0

type List struct{}

type emptyStruct struct{}

var emptyStructKey = emptyStruct{}

const stringKey = "somestring"

type customStringKeyT string

const customStringKey = customStringKeyT("customstring")

var someString = "hello"
var ptrKey *string = &someString

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

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

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

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

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

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

[cespare]

If you change

var keyInt keyIntType = 0

to

var keyInt keyIntType = 1

then it allocates. If you change

const stringKey = "somestring"

to

var stringKey = "somestring"

then it allocates. (Tested with Go 1.12.5.)

There are some nice optimizations to prevent certain interface allocations, but they shouldn't be documented here. Best to stick with simple rules of thumb.

[bcmills]

CC @Sajmani for context.

[evanj]

Wow thanks @cespare! Since I was wrong and this does matter, should we change the example in the package to follow its own advice and not use type key int?: https://github.com/golang/go/blob/master/src/context/context.go#L133

There is also duplicated description about context key types: There is a description on Context.Value: "A key can be any type that supports equality [...]", as well as on WithValue: "The provided key must be comparable and [...]" Possibly the WithValue documentation should refer to Context.Value, or vice-versa. Alternatively, both places could have the full description of "good" key types?

Here are the updated metrics with the var string and key types for Go 1.12. As you can see: var/const makes a huge difference.

Go 1.12

BenchmarkVarInterfaceKey-2           	1000000000	         2.67 ns/op	       0 B/op	       0 allocs/op
BenchmarkVarIntKeyZero-2          	300000000	         4.34 ns/op	       0 B/op	       0 allocs/op
BenchmarkVarIntKeyOne-2           	100000000	        20.6 ns/op	       8 B/op	       1 allocs/op
BenchmarkConstIntKeyOne-2         	1000000000	         2.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkConstStringKey-2         	1000000000	         2.65 ns/op	       0 B/op	       0 allocs/op
BenchmarkVarStringKey-2           	50000000	        36.5 ns/op	      16 B/op	       1 allocs/op
BenchmarkConstCustomStringKey-2   	1000000000	         2.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkVarCustomStringKey-2     	50000000	        36.2 ns/op	      16 B/op	       1 allocs/op
BenchmarkVarEmptyStructKey-2      	1000000000	         2.63 ns/op	       0 B/op	       0 allocs/op
BenchmarkVarPtrKey-2              	1000000000	         2.63 ns/op	       0 B/op	       0 allocs/op

[Sajmani]

I agree we should update the documentation to be consistent with best practices.

Even better would be to provide a library function to create a good context key, if that provides good performance:

var myKey = context.NewKey()

[mitar]

How does this compare to the comment from net/http package:

// contextKey is a value for use with context.WithValue. It's used as
// a pointer so it fits in an interface{} without allocation.
type contextKey struct {
	name string
}

So should this be the recommended way? Simply using a pointer?

Does emptyStruct{} fit in an interface{}?