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

IsFqdn performance issues #1452

Closed
lobeck opened this issue Apr 17, 2023 · 3 comments · Fixed by #1453
Closed

IsFqdn performance issues #1452

lobeck opened this issue Apr 17, 2023 · 3 comments · Fixed by #1453

Comments

@lobeck
Copy link

lobeck commented Apr 17, 2023

I have recently added the kubernetes node-cache and noticed a quite high CPU load. The cached node was actually higher than any of the load on the dns server before adding the caching layer.

So after a bit of pprof- and perf-ing around, I've noticed, that the IsFqdn call seems to be excessively expensive. Is this something that can be easily optimized? I'm not too deep into Go and the associated logic here, so feedback is welcome :D

image (11)

@tmthrgd
Copy link
Collaborator

tmthrgd commented Apr 17, 2023

It's definitely showing up in your profiles, the logic is a little tricky, but I definitely see some opportunities to optimise it.

@lobeck
Copy link
Author

lobeck commented Apr 17, 2023

Jupp, that's why I started looking, as the receiving and sending side are also somewhat expensive, but it's just plain UDP mangling.

I went deeper and did some benchmarking based on the values from the unittests, but there's nothing obvious. So, you are likely right, that it could use some optimization, especially around strings.LastIndexFunc but it's not the performance issue I was looking for. So I guess, the code is fine and "We're holding it wrong"

Screenshot 2023-04-17 at 16 24 51

BTW: This is x86 on AWS, a M1 Mac is 4x as fast.

@tmthrgd
Copy link
Collaborator

tmthrgd commented Apr 18, 2023

@lobeck Would you mind trying out #1453 and seeing if that makes a difference to your profiles?

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.

2 participants