Skip to content

Commit

Permalink
fqdn: Fix benchmarking for fqdn cache test
Browse files Browse the repository at this point in the history
Rather than testing a number of iterations of the core update function
here, the test was varying the data input size. This was violating the
recommendations from the Go docs, which state:

    The benchmark function must run the target code b.N times. During
    benchmark execution, b.N is adjusted until the benchmark function
    lasts long enough to be timed reliably.

This could lead to unreliable benchmarking numbers. Fix it.

Example run:

    $ go test ./pkg/fqdn -bench BenchmarkFqdnCache
    ...
    goos: linux
    goarch: amd64
    pkg: github.com/cilium/cilium/pkg/fqdn
    cpu: 13th Gen Intel(R) Core(TM) i7-1365U
    BenchmarkFqdnCache-12                100         110924076 ns/op
    PASS
    ok      github.com/cilium/cilium/pkg/fqdn       11.505s

Suggested-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Signed-off-by: Joe Stringer <joe@cilium.io>
  • Loading branch information
joestringer committed May 13, 2024
1 parent 900c846 commit ae9036c
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions pkg/fqdn/name_manager_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ func BenchmarkUpdateGenerateDNS(b *testing.B) {
// endpoints is. Each endpoints has 1000 DNS lookups, each with 10 IPs. The
// dump iterates over all endpoints, lookups, and IPs.
func BenchmarkFqdnCache(b *testing.B) {
caches := make([]*DNSCache, 0, b.N)
const endpoints = 8

caches := make([]*DNSCache, 0, endpoints)
for i := 0; i < b.N; i++ {
lookupTime := time.Now()
dnsHistory := NewDNSCache(0)
Expand Down Expand Up @@ -113,9 +115,11 @@ func BenchmarkFqdnCache(b *testing.B) {
return out
},
})
b.ResetTimer()

prefixMatcher := func(_ netip.Addr) bool { return true }
nameMatcher := func(_ string) bool { return true }
nameManager.GetDNSHistoryModel("", prefixMatcher, nameMatcher, "")

b.ResetTimer()
for i := 0; i < b.N; i++ {
nameManager.GetDNSHistoryModel("", prefixMatcher, nameMatcher, "")
}
}

0 comments on commit ae9036c

Please sign in to comment.