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

net: precompute rfc6724policyTable in addrselect #54032

Closed
Thorleon opened this issue Jul 24, 2022 · 3 comments
Closed

net: precompute rfc6724policyTable in addrselect #54032

Thorleon opened this issue Jul 24, 2022 · 3 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Backlog

Comments

@Thorleon
Copy link
Contributor

As net package has one of the biggest init time in standard library, I have tried to improve performance by doing two things in net/addrselect.go:

  1. Precompute slice with RFC rules. Currently the rules are computed and sorted in init() function. We could save the time and allocations by using prepopulated values in sorted manner. The rules haven't changed since 2015. To be extra safe we could move order validation as test case. It should slightly speed up startup of each binary with "net" package and go dns resolver. It also saves 38 allocations, ~50% of allocations in init phase of net module.
  2. Replace internal net.IP usage with netip.Addr in sortByRFC6724 function. It results in ~40% performance improvement on samples from tests.

The only risk is the difference between net.IP and netip.Addr behaviour.

Init benchmark:

name    old time/op    new time/op    delta
Init-8    2.14µs ± 0%    0.12µs ± 0%   ~     (p=1.000 n=1+1)

name    old alloc/op   new alloc/op   delta
Init-8    1.05kB ± 0%    0.38kB ± 0%   ~     (p=1.000 n=1+1)

name    old allocs/op  new allocs/op  delta
Init-8      39.0 ± 0%       1.0 ± 0%   ~     (p=1.000 n=1+1)

Whole sortByRFC6724 function benchmark:

name               old time/op  new time/op  delta
SortByRFC6724/0-8   479ns ± 0%   290ns ± 0%   ~     (p=1.000 n=1+1)
SortByRFC6724/1-8   482ns ± 0%   291ns ± 0%   ~     (p=1.000 n=1+1)
SortByRFC6724/2-8   489ns ± 0%   297ns ± 0%   ~     (p=1.000 n=1+1)
SortByRFC6724/3-8   580ns ± 0%   367ns ± 0%   ~     (p=1.000 n=1+1)
SortByRFC6724/4-8   934ns ± 0%   574ns ± 0%   ~     (p=1.000 n=1+1)
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/419356 mentions this issue: net: precompute rfc6724policyTable in addrselect

@ianlancetaylor
Copy link
Contributor

We should either have a test case verifying that the table is correct, or we should generate it with a go:generate command.

@cherrymui cherrymui added Performance help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 25, 2022
@cherrymui cherrymui added this to the Backlog milestone Jul 25, 2022
@Thorleon
Copy link
Contributor Author

Thorleon commented Aug 1, 2022

Sure @ianlancetaylor, I have added test for verifying table content correctness, in addition to test case with order checking.

gopherbot pushed a commit that referenced this issue Sep 5, 2022
@Thorleon @mvdan
net: precompute rfc6724policyTable in addrselect
a295890 
As net package has one of the biggest init time in standard library, I have tried to improve performance by doing two things in net/addrselect.go:
1. Precompute slice with RFC rules. Currently the rules are computed and sorted in init() function. We could save the time and allocations by using prepopulated values in sorted manner. The rules haven't changed since 2015. To be extra safe we could move order validation as test case. It should slightly speed up startup of each binary with "net" package and go dns resolver. It also saves 38 allocations, ~50% of allocations in init phase of `net` module.
2. Replace internal net.IP usage with netip.Addr in `sortByRFC6724` function. It results in ~40% performance improvement on samples from tests.

The only risk is the difference between net.IP and netip.Addr behaviour.

Init benchmark:
Init-8               1.89µs ± 2%    0.12µs ± 3%  -93.79%  (p=0.000 n=5+5)

name               old alloc/op   new alloc/op   delta
Init-8               1.05kB ± 0%    0.38kB ± 0%     ~     (zero variance)

name               old allocs/op  new allocs/op  delta
Init-8                 39.0 ± 0%       1.0 ± 0%     ~     (zero variance)

Whole sortByRFC6724 function benchmark:
name               old time/op    new time/op    delta
SortByRFC6724/0-8     463ns ± 3%     303ns ± 4%  -34.72%  (p=0.000 n=5+5)
SortByRFC6724/1-8     481ns ± 8%     306ns ± 1%  -36.46%  (p=0.000 n=5+5)
SortByRFC6724/2-8     470ns ± 4%     307ns ± 4%  -34.77%  (p=0.000 n=5+5)
SortByRFC6724/3-8     567ns ± 3%     367ns ± 3%  -35.28%  (p=0.000 n=5+5)
SortByRFC6724/4-8     918ns ± 3%     560ns ± 2%  -38.93%  (p=0.000 n=5+5)

Updates #54032

Change-Id: Ic18df1ea73805cb184c6ceb73470ca7f0b922032
Reviewed-on: https://go-review.googlesource.com/c/go/+/419356
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@ianlancetaylor @gopherbot @Thorleon @cherrymui