portlist: update some internals to use append-style APIs

In prep for reducing garbage, being able to reuse memory.  So far this
doesn't actually reuse much. This is just changing signatures around.

But some improvement in any case:

    bradfitz@tsdev:~/src/tailscale.com$ ~/go/bin/benchstat before after
    name       old time/op    new time/op    delta
    GetList-8    11.8ms ± 9%     9.9ms ± 3%  -15.98%  (p=0.000 n=10+10)

    name       old alloc/op   new alloc/op   delta
    GetList-8    99.5kB ± 2%    91.9kB ± 0%   -7.62%  (p=0.000 n=9+9)

    name       old allocs/op  new allocs/op  delta
    GetList-8     3.05k ± 1%     2.93k ± 0%   -3.83%  (p=0.000 n=8+9)

More later, once parsers can reuse strings from previous parses.

Updates #5958

Change-Id: I76cd5048246dd24d11c4e263d8bb8041747fb2b0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

portlist/netstat.go

@@ -55,7 +55,7 @@ type nothing struct{}
 // formats that we can parse without special detection logic.
 // Unfortunately, options to filter by proto or state are non-portable,
 // so we'll filter for ourselves.
-func parsePortsNetstat(output string) List {
+func appendParsePortsNetstat(base []Port, output string) []Port {
 	m := map[Port]nothing{}
 	lines := strings.Split(string(output), "\n")
 
@@ -131,13 +131,18 @@ func parsePortsNetstat(output string) List {
 		lastline = ""
 	}
 
-	l := []Port{}
+	ret := base
 	for p := range m {
-		l = append(l, p)
+		ret = append(ret, p)
 	}
-	sort.Slice(l, func(i, j int) bool {
-		return (&l[i]).lessThan(&l[j])
+
+	// Only sort the part we appended. It's up to the caller to sort the whole
+	// thing if they'd like. In practice the caller's base will have len 0,
+	// though, so the whole thing will be sorted.
+	toSort := ret[len(base):]
+	sort.Slice(toSort, func(i, j int) bool {
+		return (&toSort[i]).lessThan(&toSort[j])
 	})
 
-	return l
+	return ret
 }

portlist/netstat_exec.go

@@ -25,7 +25,7 @@ func hideWindow(c *exec.Cmd) *exec.Cmd {
 	return c
 }
 
-func listPortsNetstat(arg string) (List, error) {
+func appendListeningPortsNetstat(base []Port, arg string) ([]Port, error) {
 	exe, err := exec.LookPath("netstat")
 	if err != nil {
 		return nil, fmt.Errorf("netstat: lookup: %v", err)
@@ -40,5 +40,5 @@ func listPortsNetstat(arg string) (List, error) {
 		return nil, fmt.Errorf("netstat: %v (%q)", err, stderr)
 	}
 
-	return parsePortsNetstat(string(output)), nil
+	return appendParsePortsNetstat(base, string(output)), nil
 }

portlist/netstat_test.go

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+//go:build !ios && !js
+// +build !ios,!js
+
 package portlist
 
 import (
@@ -83,7 +86,7 @@ func TestParsePortsNetstat(t *testing.T) {
 		Port{"udp", 9353, "iTunes", ""},
 	}
 
-	pl := parsePortsNetstat(netstatOutput)
+	pl := appendParsePortsNetstat(nil, netstatOutput)
 	jgot, _ := json.MarshalIndent(pl, "", "\t")
 	jwant, _ := json.MarshalIndent(want, "", "\t")
 	if len(pl) != len(want) {

portlist/poller.go

@@ -19,13 +19,16 @@ type Poller struct {
 	// Run completes, after which Err can be checked.
 	C <-chan List
 
-	c chan List
-
 	// Err is the error from the final GetList call. It is only
 	// valid to read once C has been closed. Err is nil if Close
 	// is called or the context is canceled.
 	Err error
 
+	// scatch is memory for Poller.getList to reuse between calls.
+	scratch []Port
+
+	c chan List // the unconstrained version of the exported C above
+
 	quitCh chan struct{} // close this to force exit
 	prev   List          // most recent data
 }
@@ -45,7 +48,7 @@ func NewPoller() (*Poller, error) {
 	// Do one initial poll synchronously so we can return an error
 	// early.
 	var err error
-	p.prev, err = getList(nil)
+	p.prev, err = p.getList()
 	if err != nil {
 		return nil, err
 	}
@@ -76,7 +79,7 @@ func (p *Poller) Run(ctx context.Context) error {
 	for {
 		select {
 		case <-tick.C:
-			pl, err := getList(p.prev)
+			pl, err := p.getList()
 			if err != nil {
 				p.Err = err
 				return err

portlist/portlist.go

@@ -76,18 +76,19 @@ func (pl List) String() string {
 
 var debugDisablePortlist = envknob.RegisterBool("TS_DEBUG_DISABLE_PORTLIST")
 
-func getList(prev List) (List, error) {
+func (p *Poller) getList() (List, error) {
 	if debugDisablePortlist() {
 		return nil, nil
 	}
-	pl, err := listPorts()
+	var err error
+	p.scratch, err = appendListeningPorts(p.scratch[:0])
 	if err != nil {
 		return nil, fmt.Errorf("listPorts: %s", err)
 	}
-	pl = sortAndDedup(pl)
-	if pl.sameInodes(prev) {
+	pl := sortAndDedup(p.scratch)
+	if pl.sameInodes(p.prev) {
 		// Nothing changed, skip inode lookup
-		return prev, nil
+		return p.prev, nil
 	}
 	pl, err = addProcesses(pl)
 	if err != nil {

portlist/portlist_ios.go

@@ -14,7 +14,7 @@ import (
 
 const pollInterval = 9999 * time.Hour
 
-func listPorts() (List, error) {
+func appendListeningPorts(base []Port) ([]Port, error) {
 	return nil, errors.New("not implemented")
 }
 

portlist/portlist_js.go

@@ -8,8 +8,8 @@ import "time"
 
 const pollInterval = 365 * 24 * time.Hour
 
-func listPorts() (List, error) {
-	return nil, nil
+func appendListeningPorts(base []Port) ([]Port, error) {
+	return base, nil
 }
 
 func addProcesses(pl []Port) ([]Port, error) {

portlist/portlist_linux.go

@@ -35,11 +35,11 @@ const (
 	v4Any       = "00000000:0000"
 )
 
-func listPorts() (List, error) {
+func appendListeningPorts(base []Port) ([]Port, error) {
+	ret := base
 	if sawProcNetPermissionErr.Load() {
-		return nil, nil
+		return ret, nil
 	}
-	var ret []Port
 
 	var br *bufio.Reader
 	for _, fname := range sockfiles {

portlist/portlist_linux_test.go

@@ -132,8 +132,10 @@ func BenchmarkParsePorts(b *testing.B) {
 
 func BenchmarkListPorts(b *testing.B) {
 	b.ReportAllocs()
+	var base []Port
 	for i := 0; i < b.N; i++ {
-		_, err := listPorts()
+		var err error
+		base, err = appendListeningPorts(base[:0])
 		if err != nil {
 			b.Fatal(err)
 		}

portlist/portlist_macos.go

@@ -21,8 +21,8 @@ import (
 // We have to run netstat, which is a bit expensive, so don't do it too often.
 const pollInterval = 5 * time.Second
 
-func listPorts() (List, error) {
-	return listPortsNetstat("-na")
+func appendListeningPorts(base []Port) ([]Port, error) {
+	return appendListeningPortsNetstat(base, "-na")
 }
 
 var lsofFailed int64 // atomic bool

portlist/portlist_other.go

@@ -12,8 +12,8 @@ import "time"
 // We have to run netstat, which is a bit expensive, so don't do it too often.
 const pollInterval = 5 * time.Second
 
-func listPorts() (List, error) {
-	return listPortsNetstat("-na")
+func appendListeningPorts(base []Port) ([]Port, error) {
+	return appendListeningPortsNetstat(base, "-na")
 }
 
 func addProcesses(pl []Port) ([]Port, error) {

portlist/portlist_test.go

@@ -14,7 +14,8 @@ import (
 func TestGetList(t *testing.T) {
 	tstest.ResourceCheck(t)
 
-	pl, err := getList(nil)
+	var p Poller
+	pl, err := p.getList()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -34,7 +35,8 @@ func TestIgnoreLocallyBoundPorts(t *testing.T) {
 	defer ln.Close()
 	ta := ln.Addr().(*net.TCPAddr)
 	port := ta.Port
-	pl, err := getList(nil)
+	var p Poller
+	pl, err := p.getList()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -194,8 +196,9 @@ func TestSameInodes(t *testing.T) {
 
 func BenchmarkGetList(b *testing.B) {
 	b.ReportAllocs()
+	var p Poller
 	for i := 0; i < b.N; i++ {
-		_, err := getList(nil)
+		_, err := p.getList()
 		if err != nil {
 			b.Fatal(err)
 		}

portlist/portlist_windows.go

@@ -15,11 +15,11 @@ import (
 // Forking on Windows is insanely expensive, so don't do it too often.
 const pollInterval = 5 * time.Second
 
-func listPorts() (List, error) {
+func appendListeningPorts(base []Port) ([]Port, error) {
 	// TODO(bradfitz): stop shelling out to netstat and use the
 	// net/netstat package instead. When doing so, be sure to filter
 	// out all of 127.0.0.0/8 and not just 127.0.0.1.
-	return listPortsNetstat("-na")
+	return appendListeningPortsNetstat(base, "-na")
 }
 
 func addProcesses(pl []Port) ([]Port, error) {
@@ -31,9 +31,9 @@ func addProcesses(pl []Port) ([]Port, error) {
 	}
 	defer tok.Close()
 	if !tok.IsElevated() {
-		return listPortsNetstat("-na")
+		return appendListeningPortsNetstat(nil, "-na")
 	}
-	return listPortsNetstat("-nab")
+	return appendListeningPortsNetstat(nil, "-nab")
 }
 
 func init() {