From 5a9d977c78ff0fd1687222abe82744b47165797e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 12 Sep 2021 19:49:37 -0700 Subject: [PATCH] portlist: reduce CPU parsing portlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid splitting fields in the common case. Field splitting was 84% of the overall CPU. name old time/op new time/op delta ParsePorts-6 33.3ms ± 2% 6.3ms ± 4% -80.97% (p=0.000 n=9+10) name old alloc/op new alloc/op delta ParsePorts-6 520B ±79% 408B ± 0% -21.49% (p=0.046 n=10+8) name old allocs/op new allocs/op delta ParsePorts-6 7.00 ± 0% 7.00 ± 0% ~ (all equal) Updates tailscale/corp#2566 Signed-off-by: Brad Fitzpatrick --- portlist/portlist_linux.go | 51 +++++++++++++++++++++++++++------ portlist/portlist_linux_test.go | 43 ++++++++++++++++++++++----- 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/portlist/portlist_linux.go b/portlist/portlist_linux.go index 0f8a74cd6..904dfa952 100644 --- a/portlist/portlist_linux.go +++ b/portlist/portlist_linux.go @@ -43,8 +43,6 @@ func listPorts() (List, error) { l := []Port{} for _, fname := range sockfiles { - proto := strings.TrimSuffix(filepath.Base(fname), "6") - // Android 10+ doesn't allow access to this anymore. // https://developer.android.com/about/versions/10/privacy/changes#proc-net-filesystem // Ignore it rather than have the system log about our violation. @@ -64,7 +62,7 @@ func listPorts() (List, error) { defer f.Close() r := bufio.NewReader(f) - ports, err := parsePorts(r, proto) + ports, err := parsePorts(r, filepath.Base(fname)) if err != nil { return nil, fmt.Errorf("parsing %q: %w", fname, err) } @@ -74,7 +72,9 @@ func listPorts() (List, error) { return l, nil } -func parsePorts(r *bufio.Reader, proto string) ([]Port, error) { +// fileBase is one of "tcp", "tcp6", "udp", "udp6". +func parsePorts(r *bufio.Reader, fileBase string) ([]Port, error) { + proto := strings.TrimSuffix(fileBase, "6") var ret []Port // skip header row @@ -85,6 +85,11 @@ func parsePorts(r *bufio.Reader, proto string) ([]Port, error) { fields := make([]mem.RO, 0, 20) // 17 current fields + some future slop + wantRemote := mem.S(v4Any) + if strings.HasSuffix(fileBase, "6") { + wantRemote = mem.S(v6Any) + } + var inoBuf []byte for err == nil { line, err := r.ReadSlice('\n') @@ -95,22 +100,29 @@ func parsePorts(r *bufio.Reader, proto string) ([]Port, error) { return nil, err } + if i := fieldIndex(line, 2); i == -1 || + !mem.HasPrefix(mem.B(line).SliceFrom(i), wantRemote) { + // Fast path for not being a listener port. + continue + } + // sl local rem ... inode fields = mem.AppendFields(fields[:0], mem.B(line)) local := fields[1] rem := fields[2] inode := fields[9] + if !rem.Equal(wantRemote) { + // not a "listener" port + continue + } + // If a port is bound to localhost, ignore it. // TODO: localhost is bigger than 1 IP, we need to ignore // more things. if mem.HasPrefix(local, mem.S(v4Localhost)) || mem.HasPrefix(local, mem.S(v6Localhost)) { continue } - if !rem.Equal(mem.S(v4Any)) && !rem.Equal(mem.S(v6Any)) { - // not a "listener" port - continue - } // Don't use strings.Split here, because it causes // allocations significant enough to show up in profiles. @@ -240,3 +252,26 @@ func foreachPID(fn func(pidStr string) error) error { } } } + +// fieldIndex returns the offset in line where the Nth field (0-based) begins, or -1 +// if there aren't that many fields. Fields are separated by 1 or more spaces. +func fieldIndex(line []byte, n int) int { + skip := 0 + for i := 0; i <= n; i++ { + // Skip spaces. + for skip < len(line) && line[skip] == ' ' { + skip++ + } + if skip == len(line) { + return -1 + } + if i == n { + break + } + // Skip non-space. + for skip < len(line) && line[skip] != ' ' { + skip++ + } + } + return skip +} diff --git a/portlist/portlist_linux_test.go b/portlist/portlist_linux_test.go index 911ea91b6..3b5343092 100644 --- a/portlist/portlist_linux_test.go +++ b/portlist/portlist_linux_test.go @@ -13,10 +13,34 @@ import ( "github.com/google/go-cmp/cmp" ) +func TestFieldIndex(t *testing.T) { + tests := []struct { + in string + field int + want int + }{ + {"foo", 0, 0}, + {" foo", 0, 2}, + {"foo bar", 1, 5}, + {" foo bar", 1, 6}, + {" foo bar", 2, -1}, + {" foo bar ", 2, -1}, + {" foo bar x", 2, 10}, + {" 1: 00000000:0016 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 34062 1 0000000000000000 100 0 0 10 0", + 2, 19}, + } + for _, tt := range tests { + if got := fieldIndex([]byte(tt.in), tt.field); got != tt.want { + t.Errorf("fieldIndex(%q, %v) = %v; want %v", tt.in, tt.field, got, tt.want) + } + } +} + func TestParsePorts(t *testing.T) { tests := []struct { name string in string + file string want []Port }{ { @@ -26,6 +50,7 @@ func TestParsePorts(t *testing.T) { }, { name: "ipv4", + file: "tcp", in: `header line 0: 0100007F:0277 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 22303 1 0000000000000000 100 0 0 10 0 1: 00000000:0016 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 34062 1 0000000000000000 100 0 0 10 0 @@ -37,6 +62,7 @@ func TestParsePorts(t *testing.T) { }, { name: "ipv6", + file: "tcp6", in: ` sl local_address remote_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode 0: 00000000000000000000000001000000:0277 00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 35720 1 0000000000000000 100 0 0 10 0 1: 00000000000000000000000000000000:1F91 00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000 1000 0 142240557 1 0000000000000000 100 0 0 10 0 @@ -50,17 +76,20 @@ func TestParsePorts(t *testing.T) { }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - buf := bytes.NewBufferString(test.in) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := bytes.NewBufferString(tt.in) r := bufio.NewReader(buf) - - got, err := parsePorts(r, "tcp") + file := "tcp" + if tt.file != "" { + file = tt.file + } + got, err := parsePorts(r, file) if err != nil { t.Fatal(err) } - if diff := cmp.Diff(got, test.want, cmp.AllowUnexported(Port{})); diff != "" { + if diff := cmp.Diff(got, tt.want, cmp.AllowUnexported(Port{})); diff != "" { t.Errorf("unexpected parsed ports (-got+want):\n%s", diff) } }) @@ -91,7 +120,7 @@ func BenchmarkParsePorts(b *testing.B) { for i := 0; i < b.N; i++ { r.Seek(0, io.SeekStart) br.Reset(r) - got, err := parsePorts(br, "tcp") + got, err := parsePorts(br, "tcp6") if err != nil { b.Fatal(err) }