From 69b90742fe852cb83e0106b8f4fe36976c3ed3c8 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 12 Jan 2025 19:14:04 -0800 Subject: [PATCH] util/uniq,types/lazy,*: delete code that's now in Go std sync.OnceValue and slices.Compact were both added in Go 1.21. cmp.Or was added in Go 1.22. Updates #8632 Updates #11058 Change-Id: I89ba4c404f40188e1f8a9566c8aaa049be377754 Signed-off-by: Brad Fitzpatrick --- cmd/k8s-operator/depaware.txt | 1 - cmd/tailscaled/depaware.txt | 1 - ipn/ipnlocal/local.go | 3 +- ipn/localapi/debugderp.go | 23 +++---- types/lazy/lazy.go | 35 ----------- types/lazy/sync_test.go | 43 -------------- util/uniq/slice.go | 62 ------------------- util/uniq/slice_test.go | 102 -------------------------------- version/print.go | 5 +- version/prop.go | 3 +- version/version.go | 3 +- wgengine/magicsock/magicsock.go | 4 +- wgengine/pendopen.go | 4 +- 13 files changed, 18 insertions(+), 271 deletions(-) delete mode 100644 util/uniq/slice.go delete mode 100644 util/uniq/slice_test.go diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 0e42fe2b6..3489e5a60 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -817,7 +817,6 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/util/systemd from tailscale.com/control/controlclient+ tailscale.com/util/testenv from tailscale.com/control/controlclient+ tailscale.com/util/truncate from tailscale.com/logtail - tailscale.com/util/uniq from tailscale.com/ipn/ipnlocal+ tailscale.com/util/usermetric from tailscale.com/health+ tailscale.com/util/vizerror from tailscale.com/tailcfg+ 💣 tailscale.com/util/winutil from tailscale.com/clientupdate+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 749c3f310..4dad47421 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -406,7 +406,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/systemd from tailscale.com/control/controlclient+ tailscale.com/util/testenv from tailscale.com/ipn/ipnlocal+ tailscale.com/util/truncate from tailscale.com/logtail - tailscale.com/util/uniq from tailscale.com/ipn/ipnlocal+ tailscale.com/util/usermetric from tailscale.com/health+ tailscale.com/util/vizerror from tailscale.com/tailcfg+ 💣 tailscale.com/util/winutil from tailscale.com/clientupdate+ diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 088f1ef75..3a2a22c58 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -111,7 +111,6 @@ import ( "tailscale.com/util/syspolicy/rsop" "tailscale.com/util/systemd" "tailscale.com/util/testenv" - "tailscale.com/util/uniq" "tailscale.com/util/usermetric" "tailscale.com/version" "tailscale.com/version/distro" @@ -3346,7 +3345,7 @@ func (b *LocalBackend) clearMachineKeyLocked() error { // incoming packet. func (b *LocalBackend) setTCPPortsIntercepted(ports []uint16) { slices.Sort(ports) - uniq.ModifySlice(&ports) + ports = slices.Compact(ports) var f func(uint16) bool switch len(ports) { case 0: diff --git a/ipn/localapi/debugderp.go b/ipn/localapi/debugderp.go index 85eb031e6..dbdf5cf79 100644 --- a/ipn/localapi/debugderp.go +++ b/ipn/localapi/debugderp.go @@ -4,6 +4,7 @@ package localapi import ( + "cmp" "context" "crypto/tls" "encoding/json" @@ -81,7 +82,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { client *http.Client = http.DefaultClient ) checkConn := func(derpNode *tailcfg.DERPNode) bool { - port := firstNonzero(derpNode.DERPPort, 443) + port := cmp.Or(derpNode.DERPPort, 443) var ( hasIPv4 bool @@ -89,7 +90,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { ) // Check IPv4 first - addr := net.JoinHostPort(firstNonzero(derpNode.IPv4, derpNode.HostName), strconv.Itoa(port)) + addr := net.JoinHostPort(cmp.Or(derpNode.IPv4, derpNode.HostName), strconv.Itoa(port)) conn, err := dialer.DialContext(ctx, "tcp4", addr) if err != nil { st.Errors = append(st.Errors, fmt.Sprintf("Error connecting to node %q @ %q over IPv4: %v", derpNode.HostName, addr, err)) @@ -98,7 +99,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { // Upgrade to TLS and verify that works properly. tlsConn := tls.Client(conn, &tls.Config{ - ServerName: firstNonzero(derpNode.CertName, derpNode.HostName), + ServerName: cmp.Or(derpNode.CertName, derpNode.HostName), }) if err := tlsConn.HandshakeContext(ctx); err != nil { st.Errors = append(st.Errors, fmt.Sprintf("Error upgrading connection to node %q @ %q to TLS over IPv4: %v", derpNode.HostName, addr, err)) @@ -108,7 +109,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { } // Check IPv6 - addr = net.JoinHostPort(firstNonzero(derpNode.IPv6, derpNode.HostName), strconv.Itoa(port)) + addr = net.JoinHostPort(cmp.Or(derpNode.IPv6, derpNode.HostName), strconv.Itoa(port)) conn, err = dialer.DialContext(ctx, "tcp6", addr) if err != nil { st.Errors = append(st.Errors, fmt.Sprintf("Error connecting to node %q @ %q over IPv6: %v", derpNode.HostName, addr, err)) @@ -117,7 +118,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { // Upgrade to TLS and verify that works properly. tlsConn := tls.Client(conn, &tls.Config{ - ServerName: firstNonzero(derpNode.CertName, derpNode.HostName), + ServerName: cmp.Or(derpNode.CertName, derpNode.HostName), // TODO(andrew-d): we should print more // detailed failure information on if/why TLS // verification fails @@ -166,7 +167,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { addr = addrs[0] } - addrPort := netip.AddrPortFrom(addr, uint16(firstNonzero(derpNode.STUNPort, 3478))) + addrPort := netip.AddrPortFrom(addr, uint16(cmp.Or(derpNode.STUNPort, 3478))) txID := stun.NewTxID() req := stun.Request(txID) @@ -292,13 +293,3 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) { // issued in the first place, tell them specifically that the // cert is bad not just that the connection failed. } - -func firstNonzero[T comparable](items ...T) T { - var zero T - for _, item := range items { - if item != zero { - return item - } - } - return zero -} diff --git a/types/lazy/lazy.go b/types/lazy/lazy.go index 43325512d..c29a03db4 100644 --- a/types/lazy/lazy.go +++ b/types/lazy/lazy.go @@ -120,41 +120,6 @@ func (z *SyncValue[T]) PeekErr() (v T, err error, ok bool) { return zero, nil, false } -// SyncFunc wraps a function to make it lazy. -// -// The returned function calls fill the first time it's called, and returns -// fill's result on every subsequent call. -// -// The returned function is safe for concurrent use. -func SyncFunc[T any](fill func() T) func() T { - var ( - once sync.Once - v T - ) - return func() T { - once.Do(func() { v = fill() }) - return v - } -} - -// SyncFuncErr wraps a function to make it lazy. -// -// The returned function calls fill the first time it's called, and returns -// fill's results on every subsequent call. -// -// The returned function is safe for concurrent use. -func SyncFuncErr[T any](fill func() (T, error)) func() (T, error) { - var ( - once sync.Once - v T - err error - ) - return func() (T, error) { - once.Do(func() { v, err = fill() }) - return v, err - } -} - // TB is a subset of testing.TB that we use to set up test helpers. // It's defined here to avoid pulling in the testing package. type TB interface { diff --git a/types/lazy/sync_test.go b/types/lazy/sync_test.go index 5578eee0c..4d1278253 100644 --- a/types/lazy/sync_test.go +++ b/types/lazy/sync_test.go @@ -354,46 +354,3 @@ func TestSyncValueSetForTest(t *testing.T) { }) } } - -func TestSyncFunc(t *testing.T) { - f := SyncFunc(fortyTwo) - - n := int(testing.AllocsPerRun(1000, func() { - got := f() - if got != 42 { - t.Fatalf("got %v; want 42", got) - } - })) - if n != 0 { - t.Errorf("allocs = %v; want 0", n) - } -} - -func TestSyncFuncErr(t *testing.T) { - f := SyncFuncErr(func() (int, error) { - return 42, nil - }) - n := int(testing.AllocsPerRun(1000, func() { - got, err := f() - if got != 42 || err != nil { - t.Fatalf("got %v, %v; want 42, nil", got, err) - } - })) - if n != 0 { - t.Errorf("allocs = %v; want 0", n) - } - - wantErr := errors.New("test error") - f = SyncFuncErr(func() (int, error) { - return 0, wantErr - }) - n = int(testing.AllocsPerRun(1000, func() { - got, err := f() - if got != 0 || err != wantErr { - t.Fatalf("got %v, %v; want 0, %v", got, err, wantErr) - } - })) - if n != 0 { - t.Errorf("allocs = %v; want 0", n) - } -} diff --git a/util/uniq/slice.go b/util/uniq/slice.go deleted file mode 100644 index 4ab933a9d..000000000 --- a/util/uniq/slice.go +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -// Package uniq provides removal of adjacent duplicate elements in slices. -// It is similar to the unix command uniq. -package uniq - -// ModifySlice removes adjacent duplicate elements from the given slice. It -// adjusts the length of the slice appropriately and zeros the tail. -// -// ModifySlice does O(len(*slice)) operations. -func ModifySlice[E comparable](slice *[]E) { - // Remove duplicates - dst := 0 - for i := 1; i < len(*slice); i++ { - if (*slice)[i] == (*slice)[dst] { - continue - } - dst++ - (*slice)[dst] = (*slice)[i] - } - - // Zero out the elements we removed at the end of the slice - end := dst + 1 - var zero E - for i := end; i < len(*slice); i++ { - (*slice)[i] = zero - } - - // Truncate the slice - if end < len(*slice) { - *slice = (*slice)[:end] - } -} - -// ModifySliceFunc is the same as ModifySlice except that it allows using a -// custom comparison function. -// -// eq should report whether the two provided elements are equal. -func ModifySliceFunc[E any](slice *[]E, eq func(i, j E) bool) { - // Remove duplicates - dst := 0 - for i := 1; i < len(*slice); i++ { - if eq((*slice)[dst], (*slice)[i]) { - continue - } - dst++ - (*slice)[dst] = (*slice)[i] - } - - // Zero out the elements we removed at the end of the slice - end := dst + 1 - var zero E - for i := end; i < len(*slice); i++ { - (*slice)[i] = zero - } - - // Truncate the slice - if end < len(*slice) { - *slice = (*slice)[:end] - } -} diff --git a/util/uniq/slice_test.go b/util/uniq/slice_test.go deleted file mode 100644 index 564fc0866..000000000 --- a/util/uniq/slice_test.go +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package uniq_test - -import ( - "reflect" - "strconv" - "testing" - - "tailscale.com/util/uniq" -) - -func runTests(t *testing.T, cb func(*[]uint32)) { - tests := []struct { - // Use uint32 to be different from an int-typed slice index - in []uint32 - want []uint32 - }{ - {in: []uint32{0, 1, 2}, want: []uint32{0, 1, 2}}, - {in: []uint32{0, 1, 2, 2}, want: []uint32{0, 1, 2}}, - {in: []uint32{0, 0, 1, 2}, want: []uint32{0, 1, 2}}, - {in: []uint32{0, 1, 0, 2}, want: []uint32{0, 1, 0, 2}}, - {in: []uint32{0}, want: []uint32{0}}, - {in: []uint32{0, 0}, want: []uint32{0}}, - {in: []uint32{}, want: []uint32{}}, - } - - for _, test := range tests { - in := make([]uint32, len(test.in)) - copy(in, test.in) - cb(&test.in) - if !reflect.DeepEqual(test.in, test.want) { - t.Errorf("uniq.Slice(%v) = %v, want %v", in, test.in, test.want) - } - start := len(test.in) - test.in = test.in[:cap(test.in)] - for i := start; i < len(in); i++ { - if test.in[i] != 0 { - t.Errorf("uniq.Slice(%v): non-0 in tail of %v at index %v", in, test.in, i) - } - } - } -} - -func TestModifySlice(t *testing.T) { - runTests(t, func(slice *[]uint32) { - uniq.ModifySlice(slice) - }) -} - -func TestModifySliceFunc(t *testing.T) { - runTests(t, func(slice *[]uint32) { - uniq.ModifySliceFunc(slice, func(i, j uint32) bool { - return i == j - }) - }) -} - -func Benchmark(b *testing.B) { - benches := []struct { - name string - reset func(s []byte) - }{ - {name: "AllDups", - reset: func(s []byte) { - for i := range s { - s[i] = '*' - } - }, - }, - {name: "NoDups", - reset: func(s []byte) { - for i := range s { - s[i] = byte(i) - } - }, - }, - } - - for _, bb := range benches { - b.Run(bb.name, func(b *testing.B) { - for size := 1; size <= 4096; size *= 16 { - b.Run(strconv.Itoa(size), func(b *testing.B) { - benchmark(b, 64, bb.reset) - }) - } - }) - } -} - -func benchmark(b *testing.B, size int64, reset func(s []byte)) { - b.ReportAllocs() - b.SetBytes(size) - s := make([]byte, size) - b.ResetTimer() - for range b.N { - s = s[:size] - reset(s) - uniq.ModifySlice(&s) - } -} diff --git a/version/print.go b/version/print.go index 7d8554279..be90432cc 100644 --- a/version/print.go +++ b/version/print.go @@ -7,11 +7,10 @@ import ( "fmt" "runtime" "strings" - - "tailscale.com/types/lazy" + "sync" ) -var stringLazy = lazy.SyncFunc(func() string { +var stringLazy = sync.OnceValue(func() string { var ret strings.Builder ret.WriteString(Short()) ret.WriteByte('\n') diff --git a/version/prop.go b/version/prop.go index fee76c65f..6026d1179 100644 --- a/version/prop.go +++ b/version/prop.go @@ -9,6 +9,7 @@ import ( "runtime" "strconv" "strings" + "sync" "tailscale.com/tailcfg" "tailscale.com/types/lazy" @@ -174,7 +175,7 @@ func IsUnstableBuild() bool { }) } -var isDev = lazy.SyncFunc(func() bool { +var isDev = sync.OnceValue(func() bool { return strings.Contains(Short(), "-dev") }) diff --git a/version/version.go b/version/version.go index 5edea22ca..2add25689 100644 --- a/version/version.go +++ b/version/version.go @@ -9,6 +9,7 @@ import ( "runtime/debug" "strconv" "strings" + "sync" tailscaleroot "tailscale.com" "tailscale.com/types/lazy" @@ -117,7 +118,7 @@ func (i embeddedInfo) commitAbbrev() string { return i.commit } -var getEmbeddedInfo = lazy.SyncFunc(func() embeddedInfo { +var getEmbeddedInfo = sync.OnceValue(func() embeddedInfo { bi, ok := debug.ReadBuildInfo() if !ok { return embeddedInfo{} diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index d3075f55d..6a49f091e 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -17,6 +17,7 @@ import ( "net/netip" "reflect" "runtime" + "slices" "strconv" "strings" "sync" @@ -59,7 +60,6 @@ import ( "tailscale.com/util/ringbuffer" "tailscale.com/util/set" "tailscale.com/util/testenv" - "tailscale.com/util/uniq" "tailscale.com/util/usermetric" "tailscale.com/wgengine/capture" "tailscale.com/wgengine/wgint" @@ -2666,7 +2666,7 @@ func (c *Conn) bindSocket(ruc *RebindingUDPConn, network string, curPortFate cur } ports = append(ports, 0) // Remove duplicates. (All duplicates are consecutive.) - uniq.ModifySlice(&ports) + ports = slices.Compact(ports) if debugBindSocket() { c.logf("magicsock: bindSocket: candidate ports: %+v", ports) diff --git a/wgengine/pendopen.go b/wgengine/pendopen.go index 7db07c685..308c3ede2 100644 --- a/wgengine/pendopen.go +++ b/wgengine/pendopen.go @@ -8,6 +8,7 @@ import ( "net/netip" "runtime" "strings" + "sync" "time" "github.com/gaissmai/bart" @@ -15,7 +16,6 @@ import ( "tailscale.com/net/packet" "tailscale.com/net/tstun" "tailscale.com/types/ipproto" - "tailscale.com/types/lazy" "tailscale.com/util/mak" "tailscale.com/wgengine/filter" ) @@ -91,7 +91,7 @@ func (e *userspaceEngine) trackOpenPreFilterIn(pp *packet.Parsed, t *tstun.Wrapp var ( appleIPRange = netip.MustParsePrefix("17.0.0.0/8") - canonicalIPs = lazy.SyncFunc(func() (checkIPFunc func(netip.Addr) bool) { + canonicalIPs = sync.OnceValue(func() (checkIPFunc func(netip.Addr) bool) { // https://bgp.he.net/AS41231#_prefixes t := &bart.Table[bool]{} for _, s := range strings.Fields(`