context: Remove allocation discussion from WithValue documentation #33742

evanj · 2019-08-20T18:37:18Z

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:

go/src/context/context.go

Lines 476 to 479 in d6ffc1d

    
           // types for keys. 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.

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 #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)
		}
	})
}

The text was updated successfully, but these errors were encountered:

cespare · 2019-08-20T18:52:15Z

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 · 2019-08-20T19:07:16Z

CC @Sajmani for context.

evanj · 2019-08-20T20:20:47Z

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 · 2020-06-25T16:12:00Z

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 · 2023-10-21T20:32:30Z

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{}?

gopherbot added the Documentation label Aug 20, 2019

bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 20, 2019

bcmills added this to the Unplanned milestone Aug 20, 2019

johncming mentioned this issue Mar 11, 2020

util/httputil: use const string instead of variable int. prometheus/prometheus#6963

Closed

iand mentioned this issue Jul 23, 2021

Propagate async blockstore.AllKeysChan errors via context ipld/go-car#187

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

context: Remove allocation discussion from WithValue documentation #33742

context: Remove allocation discussion from WithValue documentation #33742

evanj commented Aug 20, 2019 •

edited by bcmills

cespare commented Aug 20, 2019

bcmills commented Aug 20, 2019

evanj commented Aug 20, 2019

Sajmani commented Jun 25, 2020

mitar commented Oct 21, 2023

context: Remove allocation discussion from WithValue documentation #33742

context: Remove allocation discussion from WithValue documentation #33742

Comments

evanj commented Aug 20, 2019 • edited by bcmills

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

What does the current documentation say

What should it say

Go 1.12

Go 1.9

Go 1.8

Test code

cespare commented Aug 20, 2019

bcmills commented Aug 20, 2019

evanj commented Aug 20, 2019

Go 1.12

Sajmani commented Jun 25, 2020

mitar commented Oct 21, 2023

evanj commented Aug 20, 2019 •

edited by bcmills

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