proposal: net/http: add Request.CopyTo

[ainar-g]

Proposal Details

The Issue

Currently, the addition of new contexts to net/http.Requests always allocates. Even when net/http.Request.WithContext is used, which creates a shallow copy of the original request with the new context, there is still the allocation of a new Request value:

func (r *Request) WithContext(ctx context.Context) *Request {
    if ctx == nil {
        panic("nil context")
    }
    r2 := new(Request)
    *r2 = *r
    r2.ctx = ctx
    return r2
}

This is a problem, since the pattern of taking the original context, creating a child context with additional data, and creating a copy of the original request with the new context is very common in HTTP middlewares.

Proposal

I propose adding a new method, net/http.Request.CopyTo:

// CopyTo writes a shallow copy of r into r2 and sets its context to ctx. The
// provided ctx and r2 must be non-nil.
//
// For outgoing client request, the context controls the entire lifetime of a
// request and its response: obtaining a connection, sending the request, and
// reading the response headers and body.
//
// To create a new request with a context, use [NewRequestWithContext]. To make
// a deep copy of a request with a new context, use [Request.Clone]. To make
// a shallow copy of a request with a new context, use [Request.WithContext].
func (r *Request) CopyTo(ctx context.Context, r2 *Request) {
    if ctx == nil {
        panic("nil context")
    } else if r2 == nil {
        panic("nil r2")
    }
    *r2 = *r
    r2.ctx = ctx
}

(The name is inspired by github.com/miekg/dns.Msg.CopyTo and is bikesheddable.)

Request.WithContext can thus be rewritten into:

func (r *Request) WithContext(ctx context.Context) *Request {
    r2 := new(Request)
    r.CopyTo(ctx, r2)
    return r2
}

This allows HTTP middlewares to reuse Request structs by using sync.Pools or some other mechanism to reuse the memory.

Related

• 44525

[gabyhelp]

Related Issues and Documentation

• net/http: avoid copy if WithContext attempts to set exact same *ctx object #44525
• net/http: Avoid copying URL in Request.WithContext #52239 (closed)
• net/http: clarify use-cases of WithContext vs Clone on requests #53413 (closed)

• [Package http > type Request ¶

       \> func (\*Request) WithContext
              ¶
  
              1.7
          ](https://go.dev/pkg/net/http/?m=old#Request.WithContext) <!-- score=0.83828 -->

• proposal: net/http: add a generic mechanism for storing arbitrary data in `Request` #60345
• proposal: net/http: NewRequest should take url.URL as target #38583 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mateusz834]

You can copy the http.Request to a sync.Pooled *http.Request, consider: This does not allocate, because WithContext gets inlined.

var sink *http.Request = new(http.Request)

func BenchmarkTest(b *testing.B) {
    r := http.Request{}
    for range b.N {
        *sink = *r.WithContext(context.Background())
    }
}

BenchmarkTest-12        76986970                15.07 ns/op            0 B/op          0 allocs/op

[ianlancetaylor]

CC @neild @bradfitz

[ainar-g]

@mateusz834, thanks! That's an interesting optimization, but it seems like it depends heavily on the surrounding function and the compiler's ability to analyse the inlinability, so anyone reading should probably put that into a separate function:

func CopyRequestTo(ctx context.Context, r *http.Request, clone *http.Request) {
    *clone = *r.WithContext(ctx)
}

[mateusz834]

@ainar-g If this gives real performance improvement we can always make sure that the WithContext gets inlined with a test, instead of adding a special API for that.

[neild]

The motivation for this proposal is performance.

I think that the first step would be to demonstrate the real-world improvement from using this API. The HTTP server is pretty allocation heavy (and the representation of headers as a map makes allocations hard to avoid), and a middleware layer which is modifying an inbound request context is probably using context.WithValue which is also allocation-heavy. Does knocking off one allocation for a new Request make a practical difference? Microbenchmarks of Request.WithContext calls are going to be less interesting than an end-to-end test of the full serving path.

[ainar-g]

@mateusz834, that would be good to have, although it also feels a bit magical.

@neild, could you please provide a good example of measuring the full serving path, so that I could measure it? I may be misunderstanding what you're asking, but as far as I can see there is no way to benchmark something like an http.Server allocating the initial request and calling the server's handler, and so on.

As for the real-world improvement, this issue has been motivated by some benchmarks I've made on a middleware covering a relatively high-load API area. http.Request.WithContext in middlewares happened to be the main cause of allocations, as http.Request is a very large structure, that could not be easily optimized away (at least not until @mateusz834's optimization proposal).

[mateusz834]

Just as a side note the current issue with WithContext exists because the escape-analysis cannot prove that the handler does not escape the *http.Request struct. Maybe instead of adding a API we can figure out a way to fix this at the compiler level?

We also have an issue #62653, which proposes a way to reduce allocations caused by interfaces, maybe the compiler can be improved with a similar idea? escape-analysis could analyse every functions with the same signature across entire app, and see whether arguments escape, not ideal but it could remove allocations in case like this, where *http.Request is probably never going to be escaped.

[neild]

@neild, could you please provide a good example of measuring the full serving path, so that I could measure it?

Make a small HTTP server serving a static response, including a middleware layer that (say) adds a context value to each request. Maybe look up the context value in the request handler, to be a bit more realistic.

Then use something like wrk or hey to send some load to it and measure the max requests/second.