xujiajun/gorouter

Benchmark is not (only) measuring the routing cost

julienschmidt opened this issue · 7 comments

Currently your benchmark code does the following:

func benchRoutes(b *testing.B, router http.Handler, routes []route) {
	b.N = 10000
	b.ReportAllocs()
	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		for _, route := range routes {
			router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(route.method, route.path, nil))
		}
	}
}

The critical part is the router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(route.method, route.path, nil)) within the loop, meaning that you allocate and initialize a new Recorder and a Request in every loop iteration.
Unfortunately that is likely much more expensive than the actual routing, meaning that both the measured memory cost and the execution time are completely meaningless, since there is a huge baseline cost.

@julienschmidt thanks your suggest. i try to modify my benchmark code does the following:

func benchRoutes(b *testing.B, router http.Handler, routes []route) {
	b.N = 10000
	b.ReportAllocs()
	b.ResetTimer()

	w := httptest.NewRecorder()
	r, _ := http.NewRequest("GET", "/", nil)

	for i := 0; i < b.N; i++ {
		for _, route := range routes {
			r.Method = route.method
			r.RequestURI = route.path
			router.ServeHTTP(w, r)
		}
	}
}

benchmark result:

➜  gorouter git:(master) ✗ go test -bench=.
GithubAPI Routes: 203
GithubAPI2 Routes: 203
   HttpRouter: 37464 Bytes
   GoRouter: 83616 Bytes
   trie-mux: 134280 Bytes
   MuxRouter: 1324192 Bytes
goos: darwin
goarch: amd64
pkg: github.com/xujiajun/gorouter
BenchmarkTrieMuxRouter-8   	   10000	    119290 ns/op	   38044 B/op	    1218 allocs/op
BenchmarkHttpRouter-8      	   10000	    101041 ns/op	   18450 B/op	     609 allocs/op
BenchmarkGoRouter-8        	   10000	    105853 ns/op	   29820 B/op	    1218 allocs/op
BenchmarkMuxRouter-8       	   10000	   4259585 ns/op	   28229 B/op	     812 allocs/op
PASS
ok  	github.com/xujiajun/gorouter	45.904s

right now? Could you give me any other advice?

Take a look at https://github.com/julienschmidt/go-http-routing-benchmark/blob/2b136956a56bc65dddfa4bdaf7d1728ae2c90d50/bench_test.go#L76
As far as I remember, it was important to set these other attributes properly too.

Btw: I don't mind if you copy some of the benchmark code, but please add proper attribution.

The b.ResetTimer() should come directly before the loop, when all preparation is done. You want to measure the execution, not the preparation.

@julienschmidt thanks your suggest.you are awesome . i try to modify my benchmark code again:

// referred to https://github.com/julienschmidt/go-http-routing-benchmark/blob/2b136956a56bc65dddfa4bdaf7d1728ae2c90d50/bench_test.go#L76
func benchRoutes(b *testing.B, router http.Handler, routes []route) {
	w := httptest.NewRecorder()
	r, _ := http.NewRequest("GET", "/", nil)
	u := r.URL
	rq := u.RawQuery

	b.N = 10000
	b.ReportAllocs()
	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		for _, route := range routes {
			r.Method = route.method
			r.RequestURI = route.path

			u.Path = route.path
			u.RawQuery = rq

			router.ServeHTTP(w, r)
		}
	}
}

benchmark result:

➜  gorouter git:(master) ✗ go test -bench=.
GithubAPI Routes: 203
GithubAPI2 Routes: 203
   HttpRouter: 37464 Bytes
   GoRouter: 83616 Bytes
   trie-mux: 134280 Bytes
   MuxRouter: 1324192 Bytes
goos: darwin
goarch: amd64
pkg: github.com/xujiajun/gorouter
BenchmarkTrieMuxRouter-8   	   10000	    127334 ns/op	   65856 B/op	     537 allocs/op
BenchmarkHttpRouter-8      	   10000	     34353 ns/op	   13792 B/op	     167 allocs/op
BenchmarkGoRouter-8        	   10000	     66331 ns/op	   13832 B/op	     406 allocs/op
BenchmarkMuxRouter-8       	   10000	   5814194 ns/op	  207171 B/op	    1760 allocs/op
PASS
ok  	github.com/xujiajun/gorouter	84.822s

BTW why should set these other attributes properly too. could you tell me the reason?

@xujiajun Mby it would be nice to include github.com/husobee/vestigo since it's a stand alone url router which is std lib compatible like your young project. Mby you can get some inspiration from it.

@negbie thanks your suggest.

@julienschmidt i have updated the REDME about benchmark: https://github.com/xujiajun/gorouter#benchmarks, any other advice?

I think the benchmark looks fine now.
However, the bench_test.go seems to be copied mostly from go-http-routing-benchmark. It should state so in the file and attribute the copyright to the original author. Strictly speaking, it would also have to be published under a compatible license, but I'm fine with the MIT License (in this case).

I'm closing this issue, as the benchmark is fixed now.