server: avoid call to trace.FromContext and resulting allocations when tracing is disabled

[dzbarsky]

Sadly trace.FromContext isn't cheap because it uses a string as the context key which leads to allocations. This patch makes the function a no-op when tracing is disabled, as it should be.

[dzbarsky]

@dfawley mind taking a look?

[dfawley]

I don't understand - how does trace.FromContext allocate memory? It uses a global var as the context key, so I don't see where the allocation would come in. I'm not opposed to this change if it shows a meaningful performance improvement, but I would like to make sure there is a real benefit first.

[dzbarsky]

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

basically a string is not a single pointer, so it can't be passed to an interface{} without an intermediate allocation.

[dzbarsky]

@dfawley ping on this one? i can show you a profile if you want :)

[dzbarsky]

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

[dfawley]

Interesting. Should net/trace be changed, then?

This is fine. I am not sure we need to do a trace.FromContext at all, even with tracing enabled, but that would be a bigger change and this is an easy quick-fix.

[dzbarsky]

Yeah i'll send net/trace a patch as well