Skip to content

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Nov 8, 2023

Pass the IP as a string to maskedIPToLabel. When called from IPStringToLabel in case of a single IP we already have the IP string and can avoid the additional allocation caused by ip.String(). For the call in the prefix case and in computeCIDRLabels there is no effect, the netip.Addr.String() call is just moved out.

name                               old time/op    new time/op    delta
IPStringToLabel/0.0.0.0/0-8          69.6ns ± 1%    71.0ns ± 2%   +1.91%  (p=0.001 n=9+10)
IPStringToLabel/192.0.2.3-8          70.9ns ± 0%    50.6ns ± 0%  -28.65%  (p=0.000 n=10+9)
IPStringToLabel/192.0.2.3/32-8       77.7ns ± 1%    78.1ns ± 1%   +0.50%  (p=0.003 n=10+10)
IPStringToLabel/192.0.2.3/24-8       77.5ns ± 0%    79.9ns ±12%   +3.02%  (p=0.000 n=10+9)
IPStringToLabel/192.0.2.0/24-8       77.5ns ± 0%    78.1ns ± 0%   +0.67%  (p=0.000 n=10+9)
IPStringToLabel/::/0-8                108ns ± 0%     109ns ± 0%     ~     (p=0.137 n=10+10)
IPStringToLabel/fdff::ff-8            136ns ± 0%      74ns ± 0%  -45.61%  (p=0.000 n=10+10)
IPStringToLabel/f00d:42::ff/128-8     155ns ± 0%     155ns ± 0%   -0.28%  (p=0.000 n=10+9)
IPStringToLabel/f00d:42::ff/96-8      145ns ± 1%     144ns ± 0%   -0.56%  (p=0.007 n=10+10)

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%     16.0B ± 0%  -50.00%  (p=0.000 n=10+10)
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            32.0B ± 0%     16.0B ± 0%  -50.00%  (p=0.000 n=10+10)
IPStringToLabel/f00d:42::ff/128-8     32.0B ± 0%     32.0B ± 0%     ~     (all equal)
IPStringToLabel/f00d:42::ff/96-8      32.0B ± 0%     32.0B ± 0%     ~     (all equal)

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%      1.00 ± 0%  -50.00%  (p=0.000 n=10+10)
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                 2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/fdff::ff-8             3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)
IPStringToLabel/f00d:42::ff/128-8      3.00 ± 0%      3.00 ± 0%     ~     (all equal)
IPStringToLabel/f00d:42::ff/96-8       2.00 ± 0%      2.00 ± 0%     ~     (all equal)

@tklauser tklauser added area/daemon Impacts operation of the Cilium daemon. kind/performance There is a performance impact of this. release-note/misc This PR makes changes that have no direct user impact. labels Nov 8, 2023
@tklauser tklauser requested a review from pippolo84 November 8, 2023 08:15
@tklauser tklauser requested review from a team as code owners November 8, 2023 08:15
@tklauser
Copy link
Member Author

tklauser commented Nov 8, 2023

/test

Pass the IP as a string to maskedIPToLabel. When called from
IPStringToLabel in case of a single IP we already have the IP string and
can avoid the additional allocation caused by ip.String(). For the call
in the prefix case and in computeCIDRLabels there is no effect, the
netip.Addr.String() call is just moved out.

name                               old time/op    new time/op    delta
IPStringToLabel/0.0.0.0/0-8          69.6ns ± 1%    71.0ns ± 2%   +1.91%  (p=0.001 n=9+10)
IPStringToLabel/192.0.2.3-8          70.9ns ± 0%    50.6ns ± 0%  -28.65%  (p=0.000 n=10+9)
IPStringToLabel/192.0.2.3/32-8       77.7ns ± 1%    78.1ns ± 1%   +0.50%  (p=0.003 n=10+10)
IPStringToLabel/192.0.2.3/24-8       77.5ns ± 0%    79.9ns ±12%   +3.02%  (p=0.000 n=10+9)
IPStringToLabel/192.0.2.0/24-8       77.5ns ± 0%    78.1ns ± 0%   +0.67%  (p=0.000 n=10+9)
IPStringToLabel/::/0-8                108ns ± 0%     109ns ± 0%     ~     (p=0.137 n=10+10)
IPStringToLabel/fdff::ff-8            136ns ± 0%      74ns ± 0%  -45.61%  (p=0.000 n=10+10)
IPStringToLabel/f00d:42::ff/128-8     155ns ± 0%     155ns ± 0%   -0.28%  (p=0.000 n=10+9)
IPStringToLabel/f00d:42::ff/96-8      145ns ± 1%     144ns ± 0%   -0.56%  (p=0.007 n=10+10)

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%     16.0B ± 0%  -50.00%  (p=0.000 n=10+10)
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            32.0B ± 0%     16.0B ± 0%  -50.00%  (p=0.000 n=10+10)
IPStringToLabel/f00d:42::ff/128-8     32.0B ± 0%     32.0B ± 0%     ~     (all equal)
IPStringToLabel/f00d:42::ff/96-8      32.0B ± 0%     32.0B ± 0%     ~     (all equal)

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%      1.00 ± 0%  -50.00%  (p=0.000 n=10+10)
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                 2.00 ± 0%      2.00 ± 0%     ~     (all equal)
IPStringToLabel/fdff::ff-8             3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)
IPStringToLabel/f00d:42::ff/128-8      3.00 ± 0%      3.00 ± 0%     ~     (all equal)
IPStringToLabel/f00d:42::ff/96-8       2.00 ± 0%      2.00 ± 0%     ~     (all equal)

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/labels-less-alloc branch from b26c125 to e4fab78 Compare November 8, 2023 08:29
@tklauser
Copy link
Member Author

tklauser commented Nov 8, 2023

/test

@tklauser tklauser changed the title labels: further optimize IPStringToLabel labels: further optimize IPStringToLabel for single IP case Nov 8, 2023
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nathanjsweet nathanjsweet merged commit 26348d3 into main Nov 8, 2023
@nathanjsweet nathanjsweet deleted the pr/tklauser/labels-less-alloc branch November 8, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. kind/performance There is a performance impact of this. 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.

3 participants