Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed two distinct problems in
bench_test.go
.Missing
Origin
header in preflight requestsThe
Origin
header is missing from the preflight request in two benchmarks:BenchmarkPreflight
andBenchmarkPreflightHeader
. As a result, those benchmarks don't really exercise the middleware's preflight logic.Re-use of the response object skews benchmarks
In a given benchmark, a
FakeResponse
object gets re-used across iterations. Accordingly, at the end of a benchmark, the response's headers contain a"Vary"
key associated to a huge slice of strings (with"Origin"
repeated many times); this is evident if you add the following statement right after the benchmark loop:Of course, because
net/http
does not re-use response objects from one handler invocation to the next, those benchmarks are not realistic. More specifically, re-using ahttp.Header
map across benchmark iterations yields skewed results, because the cost of adding an element to the slice of strings associated to key"Vary"
is amortised byappend
(whichhttp.Header.Add
uses under the hood). Therefore, the benchmark results look better than they realistically should.A simple solution is to clear the
FakeResponse.header
map at the beginning of each benchmark iteration. I've added a small helper to do that, while we wait for the newclear
builtin promised by Go 1.21.New benchmark results
I have not updated the Benchmarks section of the README (you may want to do that yourself after running the benchmarks on your own machine), but here are the results I typically get on my old Macbook Pro: