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

Optimize GetControllerName for CNP #23717

Merged
merged 1 commit into from Mar 9, 2023

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Feb 13, 2023

Use strings.Builder instead of fmt.Sprintf().

Results:

benchmark                            old ns/op     new ns/op     delta
BenchmarkCNPGetControllerName-10     338           206           -39.05%

benchmark                            old allocs     new allocs     delta
BenchmarkCNPGetControllerName-10     6              4              -33.33%

benchmark                            old bytes     new bytes     delta
BenchmarkCNPGetControllerName-10     464           432           -6.90%

Signed-off-by: Marcel Zięba marseel@gmail.com
Related: #19571

@marseel marseel requested review from a team as code owners February 13, 2023 15:09
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 13, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Feb 13, 2023
@marseel marseel force-pushed the optimize_get_controller_name branch 3 times, most recently from cf64b52 to 01b440c Compare February 23, 2023 12:14
@maintainer-s-little-helper
Copy link

Commit 01b440c75afd0045e579183169160a2ac576a9c8 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Feb 23, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Feb 23, 2023
@marseel
Copy link
Contributor Author

marseel commented Feb 24, 2023

/test

@sayboras
Copy link
Member

Can you help to rebase with master, so that the latest ConformanceGatewayAPI tests can be picked up? Thanks.

Use strings.Builder instead of fmt.Sprintf().

Results:
```
benchmark                            old ns/op     new ns/op     delta
BenchmarkCNPGetControllerName-10     338           206           -39.05%

benchmark                            old allocs     new allocs     delta
BenchmarkCNPGetControllerName-10     6              4              -33.33%

benchmark                            old bytes     new bytes     delta
BenchmarkCNPGetControllerName-10     464           432           -6.90%
```

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@sayboras sayboras added the release-note/misc This PR makes changes that have no direct user impact. label Feb 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 28, 2023
@aanm
Copy link
Member

aanm commented Mar 6, 2023

/test

@aanm aanm merged commit ce1f94e into cilium:master Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants