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.

Sign up for GitHub

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

Jump to bottom

fqdn: Fix notifyOnDNSMsg benchmark #32454

Merged
merged 2 commits into from
May 21, 2024
Merged

fqdn: Fix notifyOnDNSMsg benchmark #32454

merged 2 commits into from
May 21, 2024

Commits on May 14, 2024

  1. fqdn: Add default values for DNSProxyLock{Count,Timeout} 

    Add defaults values for DNSProxyLockCount and DNSProxyLockTimeout in
order to refer them both in the global options initialization and in the
notifyOnDNSMessage benchmark.

Signed-off-by: Fabio Falzoi <[email protected]>
    @pippolo84
    pippolo84 committed May 14, 2024
    Configuration menu
    Copy the full SHA
    66ab1d0 View commit details
    Browse the repository at this point in the history

  2. fqdn: Fix notifyOnDNSMsg benchmark 

    The design of BenchmarkNotifyOnDNSMsg was flawed due to the usage of b.N
as benchmark input size. b.N is used by the Go benchmarking framework to
find a reliable timing for the code under test. Changing the amount of
work done at each benchmark call leads to unreliable numbers.  For more
info refer to the Go documentation:

	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.

And the section `Traps for young players` in the blog post
https://dave.cheney.net/2013/06/30/how-to-write-benchmarks-in-go

To fix it, the benchmark has been updated to setup a reasonable fixed
number of endpoints.

Also, since the code was behind the latest development in the policy and
fqdn subsystems, it has been updated to run correctly. Specifically:

- the global LocalNodeStore is set once at startup
- the FQDN selectors are added to the selector cache with a dummy user
- the daemon context is properly initialized
- the FQDN related global opts are set to their defaults to avoid noisy
  logs

Example run:

goos: linux
goarch: amd64
pkg: github.com/cilium/cilium/daemon/cmd
cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
BenchmarkNotifyOnDNSMsg-8   	      21	  47992143 ns/op	30757230 B/op	  357646 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      24	  48773334 ns/op	30590877 B/op	  355639 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      22	  47940489 ns/op	30719252 B/op	  358088 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      24	  48019454 ns/op	30364074 B/op	  353470 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      24	  66607283 ns/op	30634838 B/op	  356352 allocs/op
PASS

Signed-off-by: Fabio Falzoi <[email protected]>
    @pippolo84
    pippolo84 committed May 14, 2024
    Configuration menu
    Copy the full SHA
    2aa79d5 View commit details
    Browse the repository at this point in the history