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/netip: optimize performance of String and MarshalText for AddrPort #63605

Closed
aimuz opened this issue Oct 18, 2023 · 4 comments
Closed

net/netip: optimize performance of String and MarshalText for AddrPort #63605

aimuz opened this issue Oct 18, 2023 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@aimuz
Copy link
Contributor

aimuz commented Oct 18, 2023 

func (p AddrPort) String() string {
	switch p.ip.z {
	case z0:
		return "invalid AddrPort"
	case z4:
		a := p.ip.As4()
		buf := make([]byte, 0, 21)
		for i := range a {
			buf = strconv.AppendUint(buf, uint64(a[i]), 10)
			buf = append(buf, "...:"[i])
		}
		buf = strconv.AppendUint(buf, uint64(p.port), 10)
		return string(buf)
	default:
		// TODO: this could be more efficient allocation-wise:
		return joinHostPort(p.ip.String(), itoa.Itoa(int(p.port)))
	}
}

func joinHostPort(host, port string) string {
	// We assume that host is a literal IPv6 address if host has
	// colons.
	if bytealg.IndexByteString(host, ':') >= 0 {
		return "[" + host + "]:" + port
	}
	return host + ":" + port
}

The current code suggests that the implementation can be simply optimized for better performance

Probably this code improvement is not important, if it should be fixed please let me know and I'll be happy to finish the job!
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 20, 2023
@cagedmantis cagedmantis added this to the Backlog milestone Oct 20, 2023
@cagedmantis cagedmantis changed the title net/netip: Optimize performance of String and MarshalText for AddrPort net/netip: optimize performance of String and MarshalText for AddrPort Oct 20, 2023
@cagedmantis
Copy link
Contributor

cc @ianlancetaylor @neild

@mauri870
Copy link
Member

mauri870 commented Oct 20, 2023 •
edited

I ran a simple benchmark with go test -benchmem -run=^$ -bench ^BenchmarkAddrPortString$ -memprofile=mem.pprof net/netip and there are 3 allocs in the joinHostPort branch. The mem profile for alloc_objects should be a good glimpse on which functions to focus on:

45510840	34.88%	34.88%	104756867	80.29%	net/netip.joinHostPort	
31665777	24.27%	59.15%	31665777	24.27%	net/netip.Addr.string6	(inline)
25712545	19.71%	78.86%	130469412	100.00%	net/netip.AddrPort.String	
14844130	11.38%	90.24%	14844130	11.38%	internal/itoa.Uitoa	(inline)
12736120	9.76%	100.00%	44401897	34.03%	net/netip.Addr.String	
0	0.00%	100.00%	14844130	11.38%	internal/itoa.Itoa

aimuz added a commit to aimuz/go that referenced this issue Nov 17, 2023
@aimuz
net/netip: optimize AddrPort.String() method
b61614f 
Improve the efficiency of AddrPort.String() by refactoring the
method to use a shared buffer and reducing memory allocations.
Performance benchmarks show significant improvements in speed
and reduction in bytes per operation, especially for IPv6 addresses.

Fixes golang#63605

Benchmark:

```
goos: darwin
goarch: amd64
pkg: net/netip
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
                                   │   old.out    │               new.out               │
                                   │    sec/op    │   sec/op     vs base                │
AddrPortString/v4-12                  72.61n ± 9%   54.14n ± 5%  -25.43% (p=0.000 n=10)
AddrPortString/v6-12                  204.2n ± 3%   116.2n ± 1%  -43.10% (p=0.000 n=10)
AddrPortString/v6_ellipsis-12         190.8n ± 1%   123.3n ± 5%  -35.35% (p=0.000 n=10)
AddrPortString/v6_v4-12              145.50n ± 4%   64.35n ± 3%  -55.77% (p=0.000 n=10)
AddrPortString/v6_zone-12             187.0n ± 1%   121.4n ± 4%  -35.06% (p=0.000 n=10)
AddrPortMarshalText/v4-12             55.74n ± 3%   54.37n ± 1%   -2.48% (p=0.003 n=10)
AddrPortMarshalText/v6-12             120.2n ± 4%   115.9n ± 1%   -3.54% (p=0.000 n=10)
AddrPortMarshalText/v6_ellipsis-12    126.1n ± 2%   122.5n ± 2%   -2.85% (p=0.001 n=10)
AddrPortMarshalText/v6_v4-12          66.34n ± 1%   65.10n ± 1%   -1.87% (p=0.000 n=10)
AddrPortMarshalText/v6_zone-12        123.3n ± 2%   123.9n ± 1%        ~ (p=0.753 n=10)
geomean                               118.2n        90.72n       -23.24%

                                   │   old.out   │               new.out                │
                                   │    B/op     │    B/op     vs base                  │
AddrPortString/v4-12                  24.00 ± 0%   24.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortString/v6-12                 101.00 ± 0%   48.00 ± 0%  -52.48% (p=0.000 n=10)
AddrPortString/v6_ellipsis-12         61.00 ± 0%   32.00 ± 0%  -47.54% (p=0.000 n=10)
AddrPortString/v6_v4-12               61.00 ± 0%   32.00 ± 0%  -47.54% (p=0.000 n=10)
AddrPortString/v6_zone-12             61.00 ± 0%   32.00 ± 0%  -47.54% (p=0.000 n=10)
AddrPortMarshalText/v4-12             24.00 ± 0%   24.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6-12             64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_ellipsis-12    64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_v4-12          64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_zone-12        64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
geomean                               54.27        41.51       -23.50%
¹ all samples are equal

                                   │  old.out   │               new.out                │
                                   │ allocs/op  │ allocs/op   vs base                  │
AddrPortString/v4-12                 1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortString/v6-12                 3.000 ± 0%   1.000 ± 0%  -66.67% (p=0.000 n=10)
AddrPortString/v6_ellipsis-12        3.000 ± 0%   1.000 ± 0%  -66.67% (p=0.000 n=10)
AddrPortString/v6_v4-12              3.000 ± 0%   1.000 ± 0%  -66.67% (p=0.000 n=10)
AddrPortString/v6_zone-12            3.000 ± 0%   1.000 ± 0%  -66.67% (p=0.000 n=10)
AddrPortMarshalText/v4-12            1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6-12            1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_ellipsis-12   1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_v4-12         1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_zone-12       1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                              1.552        1.000       -35.56%
¹ all samples are equal
```
@aimuz aimuz mentioned this issue Nov 17, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/543177 mentions this issue: net/netip: optimize AddrPort.String() method

aimuz added a commit to aimuz/go that referenced this issue Nov 18, 2023
@aimuz
net/netip: optimize AddrPort.String() method
73285ea 
Improve the efficiency of AddrPort.String() by refactoring the
method to use a shared buffer and reducing memory allocations.
Performance benchmarks show significant improvements in speed
and reduction in bytes per operation, especially for IPv6 addresses.

Fixes golang#63605

```
benchstat old.out new.out
goos: darwin
goarch: amd64
pkg: net/netip
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
                                   │    old.out    │               new.out               │
                                   │    sec/op     │   sec/op     vs base                │
AddrPortString/v4-12                  49.24n ±  1%   50.65n ± 1%   +2.87% (p=0.000 n=10)
AddrPortString/v6-12                  165.9n ±  1%   113.7n ± 3%  -31.43% (p=0.000 n=10)
AddrPortString/v6_ellipsis-12         160.5n ±  1%   113.0n ± 2%  -29.56% (p=0.000 n=10)
AddrPortString/v6_v4-12              119.00n ±  3%   57.52n ± 4%  -51.66% (p=0.000 n=10)
AddrPortString/v6_zone-12             164.0n ±  1%   111.8n ± 1%  -31.86% (p=0.000 n=10)
AddrPortMarshalText/v4-12             51.52n ±  3%   51.02n ± 0%        ~ (p=0.159 n=10)
AddrPortMarshalText/v6-12             117.6n ±  3%   116.2n ± 1%   -1.19% (p=0.012 n=10)
AddrPortMarshalText/v6_ellipsis-12    119.5n ±  3%   118.0n ± 1%        ~ (p=0.085 n=10)
AddrPortMarshalText/v6_v4-12          63.55n ± 14%   63.25n ± 2%        ~ (p=0.739 n=10)
AddrPortMarshalText/v6_zone-12        115.7n ±  1%   117.4n ± 3%   +1.47% (p=0.003 n=10)
geomean                               103.1n         85.85n       -16.77%

                                   │  old.out   │               new.out                │
                                   │    B/op    │    B/op     vs base                  │
AddrPortString/v4-12                 24.00 ± 0%   24.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortString/v6-12                 96.00 ± 0%   48.00 ± 0%  -50.00% (p=0.000 n=10)
AddrPortString/v6_ellipsis-12        56.00 ± 0%   32.00 ± 0%  -42.86% (p=0.000 n=10)
AddrPortString/v6_v4-12              56.00 ± 0%   32.00 ± 0%  -42.86% (p=0.000 n=10)
AddrPortString/v6_zone-12            56.00 ± 0%   32.00 ± 0%  -42.86% (p=0.000 n=10)
AddrPortMarshalText/v4-12            24.00 ± 0%   24.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6-12            64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_ellipsis-12   64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_v4-12         64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_zone-12       64.00 ± 0%   64.00 ± 0%        ~ (p=1.000 n=10) ¹
geomean                              52.63        41.51       -21.12%
¹ all samples are equal

                                   │  old.out   │               new.out                │
                                   │ allocs/op  │ allocs/op   vs base                  │
AddrPortString/v4-12                 1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortString/v6-12                 2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=10)
AddrPortString/v6_ellipsis-12        2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=10)
AddrPortString/v6_v4-12              2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=10)
AddrPortString/v6_zone-12            2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=10)
AddrPortMarshalText/v4-12            1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6-12            1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_ellipsis-12   1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_v4-12         1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
AddrPortMarshalText/v6_zone-12       1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                              1.320        1.000       -24.21%
¹ all samples are equal
```
@aimuz
Copy link
Contributor Author

aimuz commented Jan 30, 2024

Repeat fix, close

@aimuz aimuz closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

net/netip: optimize AddrPort.String() method aimuz/go
4 participants
@cagedmantis @gopherbot @mauri870 @aimuz