Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Middleware benchmarks are still skewed (follow-up to #156) #165

Closed
jub0bs opened this issue Feb 9, 2024 · 0 comments · Fixed by #166
Closed

Middleware benchmarks are still skewed (follow-up to #156) #165

jub0bs opened this issue Feb 9, 2024 · 0 comments · Fixed by #166

Comments

@jub0bs
Copy link
Contributor

jub0bs commented Feb 9, 2024

Unfortunately, the benchmarks are still skewed, even after #156. Sorry I didn't realise this at the time of the PR in question.

Problem

In a given benchmark, a FakeResponse object gets re-used across iterations. At the start of each iteration, its header field (of type http.Header) is cleared. However, the clear function empties the map without reducing its capacity. Therefore, the cost (esp. in terms of heap allocations) of re-adding headers to the shared FakeResponse object in subsequent iterations is amortised.

Of course, net/http does not re-use response objects from one handler invocation to the next. Therefore, the benchmark results look better than they realistically should.

One solution

One way to fix the issue is to use a different FakeResponse object for each iteration. You don't want to account for the initialisation of each FakeResponse in the benchmark results, though. One possible approach consists in creating a collection of FakeResponses (one for each benchmark iteration) before starting the benchmark loop.

func BenchmarkWithout(b *testing.B) {
	resps := makeFakeResponses(b.N)
	req, _ := http.NewRequest(http.MethodGet, dummyEndpoint, nil)

	b.ReportAllocs()
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		testHandler.ServeHTTP(resps[i], req)
	}
}

func makeFakeResponses(n int) []*FakeResponse {
	resps := make([]*FakeResponse, n)
	for i := 0; i < n; i++ {
		resps[i] = &FakeResponse{http.Header{}}
	}
	return resps
}

New benchmark results:

$ go test -benchmem -run '^$' -bench .

goos: darwin
goarch: amd64
pkg: github.com/rs/cors
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
BenchmarkWithout-8           	269766921	        20.93 ns/op	       0 B/op	       0 allocs/op
BenchmarkDefault-8           	 3944059	       268.4 ns/op	     352 B/op	       1 allocs/op
BenchmarkAllowedOrigin-8     	 4456435	       285.8 ns/op	     352 B/op	       1 allocs/op
BenchmarkPreflight-8         	 2806293	       425.9 ns/op	     352 B/op	       1 allocs/op
BenchmarkPreflightHeader-8   	 2219377	       517.5 ns/op	     352 B/op	       1 allocs/op
@jub0bs jub0bs changed the title Fix skewed middleware benchmarks (follow-up to #156) Middleware benchmarks are still skewed (follow-up to #156) Feb 9, 2024
@rs rs closed this as completed in #166 Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant