Change https://golang.org/cl/335790 mentions this issue: context: implement Context.Value using iteration rather than recursion

Change https://golang.org/cl/335789 mentions this issue: context: add benchmarks for Value with a long chain

It seems to me that a context chain 12 long is the anomaly here. Why is it so long? That, to me, is the real problem here. We shouldn't rewrite every recursive call because in some cases the stack can grow. That removes recursion as a tool, and recursion is too elegant a tool to be castigated.

I am not in favor of going down this road.

I'm not sure how to measure the lengths of context chains in my own projects, but don't find 12 to be an anomaly at all; contexts are frequently used for tracing/metrics/logging, where the length of the chain corresponds to the number of interesting calls and logged values. Throw in some WithTimeout/WithDeadline/WithCancel, and it's definitely above 12 on average for my service.

Good to know. Thanks.

I still feel that rewriting library code to avoid recursion because it's sometimes expensive is not wise practice. Certainly not a precedent to establish.

By the way, the screenshot link above is Google-internal, I think.

The context.WithValue API encourages the creation of long chains. (Note that #33283 discusses the lack of advice on how context.WithValue should be used.)

In the case of the server I am talking about, I don't think the application logic creates any of the value contexts -- they are all created at the framework level.

A cropped view of the flame graph:

Here is an example from another server with a chain of 26 contexts:

I have been pointed to other services with chains of over 50.

I managed to print a context for a server which handles HTTP traffic where the length of the chain was 19:

• 19 contexts:
  • 1 context.Background()
  • 2 context.WithCancel()
  • 16 context.WithValue()
    • 5 were created by the standard library with keys:
      • http.ServerContextKey
      • http.LocalAddrContextKey
      • pprof.labelContextKey{}
      • trace.traceContextKey{} * 2
    • 4 were created by a server framework
    • 3 were created by an experiments framework
    • 1 was created by the application logic
    • 3 were created by other things

So the standard library "owns" 8 out of 19 links in the chain.

There doesn't seem to be a natural limit to the size of a context chain, and it's clear that long chains occur in practice.

While I agree that we shouldn't avoid recursion as a general principle, it doesn't seem like a recursive traversal of an arbitrarily-long linked list is the obviously correct approach.

It's correct, just not the fastest.

I am persuaded that a careful iterative approach is warranted here, provided it does not encourage a broad rewrite of recursive code in the libraries.

Still, it bothers me that a depth of 12 or 19 or even 50 is considered problematic for recursion, especially in Go, which handles deep recursion better than many languages.

Still, it bothers me that a depth of 12 or 19 or even 50 is considered problematic for recursion, especially in Go, which handles deep recursion better than many languages.

That is because it resizes the stack rather than hitting a stack overflow. For long lived goroutines the resize cost is amortized so does not matter.

But for short lived goroutines that cost can be significant.

Off topic: What I would really like is to have the option of starting sensitive goroutines with an initial stacksize chosen based on past history. Doing that might look like

var runner = smartstacksize.New()

func foo(...) {
  runner.Go(func() { ... })
  ...
}

One could build such a runner if there was a runtime.CurrentStacksize() and runtime.GoWithStackHint().

See: #18138

The CL https://go-review.googlesource.com/c/go/+/335790 has two thumbs-up reviews and one thumbs-up for trust. Is there anything that is holding it back from merging?