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

v1.13 Backports 2023-11-02 #28950

Merged
merged 2 commits into from Nov 6, 2023
Merged

Conversation

pippolo84
Copy link
Member

@pippolo84 pippolo84 commented Nov 2, 2023

Once this PR is merged, a GitHub action will update the labels of these PRs:

 28649 28647

@pippolo84 pippolo84 added kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels Nov 2, 2023
[ upstream commit d6e3eda ]

As suggested by package "sort" documentation
(https://pkg.go.dev/sort#Strings), slices.Sort should be preferred to
sort.Strings since it runs faster.

Comparison between sort.Strings ("old") and slices.Sort ("new"):

name                        old time/op    new time/op    delta
pkg:github.com/cilium/cilium/pkg/labels goos:linux goarch:amd64
Labels_SortedList-8            787ns ± 4%     736ns ±12%   -6.42%  (p=0.014 n=7+8)
pkg:github.com/cilium/cilium/pkg/labels/cidr goos:linux goarch:amd64
Labels_SortedListCIDRIDs-8    5.22µs ± 8%    4.67µs ± 4%  -10.63%  (p=0.000 n=10+10)

name                        old alloc/op   new alloc/op   delta
pkg:github.com/cilium/cilium/pkg/labels goos:linux goarch:amd64
Labels_SortedList-8             360B ± 0%      336B ± 0%   -6.67%  (p=0.000 n=10+10)
pkg:github.com/cilium/cilium/pkg/labels/cidr goos:linux goarch:amd64
Labels_SortedListCIDRIDs-8    1.62kB ± 0%    1.60kB ± 0%   -1.48%  (p=0.000 n=10+10)

name                        old allocs/op  new allocs/op  delta
pkg:github.com/cilium/cilium/pkg/labels goos:linux goarch:amd64
Labels_SortedList-8             3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)
pkg:github.com/cilium/cilium/pkg/labels/cidr goos:linux goarch:amd64
Labels_SortedListCIDRIDs-8      3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
[ upstream commit b448f92 ]

When generating labels for IPv6 addresses, ':' characters need to be
replaced with '-'. Also, a leading or trailing '0' is added to avoid
labels starting or ending with '-'.  To avoid the allocation of an
additional string, replace the character on the fly while accumulating
the label key in the strings.Builder.

IPStringToLabel/0.0.0.0/0-8           104ns ±11%      94ns ± 4%  -10.25%  (p=0.008 n=5+5)
IPStringToLabel/192.0.2.3-8           146ns ± 3%     128ns ±27%     ~     (p=0.841 n=5+5)
IPStringToLabel/192.0.2.3/32-8        175ns ±11%     190ns ± 8%     ~     (p=0.056 n=5+5)
IPStringToLabel/192.0.2.3/24-8        176ns ±12%     167ns ± 7%     ~     (p=0.095 n=5+5)
IPStringToLabel/192.0.2.0/24-8        169ns ± 2%     162ns ± 1%   -4.08%  (p=0.008 n=5+5)
IPStringToLabel/::/0-8                309ns ± 2%     232ns ±11%  -24.91%  (p=0.008 n=5+5)
IPStringToLabel/fdff::ff-8            412ns ± 1%     264ns ± 1%  -35.90%  (p=0.008 n=5+5)
IPStringToLabel/f00d:42::ff/128-8     469ns ± 2%     305ns ± 1%  -34.96%  (p=0.016 n=5+4)
IPStringToLabel/f00d:42::ff/96-8      405ns ±11%     293ns ± 7%  -27.58%  (p=0.008 n=5+5)

name                               old alloc/op   new alloc/op   delta
IPStringToLabel/0.0.0.0/0-8           24.0B ± 0%     24.0B ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.3-8           32.0B ± 0%     32.0B ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.3/32-8        32.0B ± 0%     32.0B ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.3/24-8        32.0B ± 0%     32.0B ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.0/24-8        32.0B ± 0%     32.0B ± 0%     ~     (all equal)
IPStringToLabel/::/0-8                16.0B ± 0%     16.0B ± 0%     ~     (all equal)
IPStringToLabel/fdff::ff-8            56.0B ± 0%     32.0B ± 0%  -42.86%  (p=0.008 n=5+5)
IPStringToLabel/f00d:42::ff/128-8     80.0B ± 0%     32.0B ± 0%  -60.00%  (p=0.008 n=5+5)
IPStringToLabel/f00d:42::ff/96-8      48.0B ± 0%     32.0B ± 0%  -33.33%  (p=0.008 n=5+5)

name                               old allocs/op  new allocs/op  delta
IPStringToLabel/0.0.0.0/0-8            2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.3-8            2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.3/32-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.3/24-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/192.0.2.0/24-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/::/0-8                 3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)
IPStringToLabel/fdff::ff-8             5.00 ± 0%      3.00 ± 0%  -40.00%  (p=0.008 n=5+5)
IPStringToLabel/f00d:42::ff/128-8      5.00 ± 0%      3.00 ± 0%  -40.00%  (p=0.008 n=5+5)
IPStringToLabel/f00d:42::ff/96-8       3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@pippolo84
Copy link
Member Author

/test-backport-1.13

@pippolo84 pippolo84 marked this pull request as ready for review November 2, 2023 16:21
@pippolo84 pippolo84 requested review from a team as code owners November 2, 2023 16:21
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 6, 2023
@aanm aanm merged commit ab5c7b2 into cilium:v1.13 Nov 6, 2023
60 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants