Add basic overhead benchmarks #46
Conversation
It turns out we do not need request.Clone, so just do a shallow copy instead. We can grind overhead further by pooling allocations to ammortize the cost.
TCN-1894 Benchmarks
We'll want benchmarks, to measure the overhead of this library and be able to compare against baseline "net/http". And we may also want a separate benchmark-like harness, to do high-throughput, high-concurrency testing, to try to tickle concurrency bugs and verify correctness in the face of traffic and many goroutines. |
Forgot about linting completely. Tunnel vision.
jchadwick-buf
changed the title
overhead_test.go
Outdated
| req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, "http://localhost:0/", nil) | ||
| resp, _ := client.Do(req) |
There was a problem hiding this comment.
Here and also below, since we don't expect any errors, would be nice to assert that with require.NoError(b, err) instead of just ignoring. (Maybe that would have also caught the issue you had before, where the request was never even making it to the underlying transport?)
There was a problem hiding this comment.
The problem with doing that is that require.NoError inadvertently takes up most of the CPU time in the test (figures shoot up to 1700-1800ns/op from around 300-400ns/op.) I can make a manual assertion, which I think is the best we can do.
There was a problem hiding this comment.
Did that. The manual assertion adds basically noise per op, so I think that's the best we can really do.
The intent of this benchmark is not to show off the performance of
httplb, but instead the overhead. Since under the hoodhttplbstill usesnet/httpand all of its primitives, the baseline performance is dictated by hownet/httpperforms. By comparing a very similar codepath throughhttplb.Clientandnet/http.Client, we can see roughly how much overhead you get by usinghttplb.Locally, I get:
...Which is not too bad, all things considered. This is obviously the minimal overhead you can get, but it seems like a good start nonetheless. More benchmarks would be nice (e.g. benchmarking TLS overhead; may want a
sync.Poolfor when we have to clone that, as well!) and I assume there's still more we could grind down in terms of CPU overhead if we wanted to. There are some events that would cause additional contention in the hot path, but these should not be happening often enough to impact the overall throughput or latency of the system by a large amount.