From e28b7b57efa5cdd286dbb08eb5bf3656a38004b5 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 6 Oct 2026 08:52:11 -0700 Subject: [PATCH] types/logid: optimize Compare (#21661) Use bytes.Compare which is up to 10x faster since it is written in assembly. Updates tailscale/corp#37735 Signed-off-by: Joe Tsai --- types/logid/id.go | 5 +- types/logid/id_test.go | 109 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/types/logid/id.go b/types/logid/id.go index 94e363879..23fc87ddd 100644 --- a/types/logid/id.go +++ b/types/logid/id.go @@ -12,7 +12,6 @@ import ( "encoding/hex" "fmt" "math/bits" - "slices" "unicode/utf8" ) @@ -66,7 +65,7 @@ func (id1 PrivateID) Less(id2 PrivateID) bool { } func (id1 PrivateID) Compare(id2 PrivateID) int { - return slices.Compare(id1[:], id2[:]) + return bytes.Compare(id1[:], id2[:]) } func (id PrivateID) IsZero() bool { @@ -116,7 +115,7 @@ func (id1 PublicID) Less(id2 PublicID) bool { } func (id1 PublicID) Compare(id2 PublicID) int { - return slices.Compare(id1[:], id2[:]) + return bytes.Compare(id1[:], id2[:]) } func (id PublicID) IsZero() bool { diff --git a/types/logid/id_test.go b/types/logid/id_test.go index 86a736bd8..29250a667 100644 --- a/types/logid/id_test.go +++ b/types/logid/id_test.go @@ -4,7 +4,9 @@ package logid import ( + "bytes" "math" + "slices" "testing" "tailscale.com/tstest" @@ -80,6 +82,113 @@ func TestIDs(t *testing.T) { } } +func TestCompare(t *testing.T) { + // Ordering is unsigned and lexicographic. Each pair is checked in both + // directions. bytes.Compare must match slices.Compare on these IDs so the + // switch does not change Compare or Less. + tests := []struct { + a, b string + want int + }{{ + a: "0000000000000000000000000000000000000000000000000000000000000000", + b: "0000000000000000000000000000000000000000000000000000000000000000", + want: 0, + }, { + a: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + b: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + want: 0, + }, { + a: "0000000000000000000000000000000000000000000000000000000000000000", + b: "0000000000000000000000000000000000000000000000000000000000000001", + want: -1, + }, { + a: "00ff000000000000000000000000000000000000000000000000000000000000", + b: "0100000000000000000000000000000000000000000000000000000000000000", + want: -1, + }, { + a: "7f00000000000000000000000000000000000000000000000000000000000000", + b: "8000000000000000000000000000000000000000000000000000000000000000", + want: -1, + }, { + a: "000000000000000000000000000000007f000000000000000000000000000000", + b: "0000000000000000000000000000000080000000000000000000000000000000", + want: -1, + }, { + a: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe", + b: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + want: -1, + }} + for _, tt := range tests { + for _, swap := range []bool{false, true} { + aStr, bStr, want := tt.a, tt.b, tt.want + if swap { + aStr, bStr, want = tt.b, tt.a, -tt.want + } + privA := must.Get(ParsePrivateID(aStr)) + privB := must.Get(ParsePrivateID(bStr)) + pubA := must.Get(ParsePublicID(aStr)) + pubB := must.Get(ParsePublicID(bStr)) + + if got, other := bytes.Compare(privA[:], privB[:]), slices.Compare(privA[:], privB[:]); got != other { + t.Errorf("bytes.Compare(%s, %s) = %d, slices.Compare = %d", aStr, bStr, got, other) + } + if got := privA.Compare(privB); got != want { + t.Errorf("PrivateID(%s).Compare(%s) = %d, want %d", aStr, bStr, got, want) + } else if got != bytes.Compare(privA[:], privB[:]) { + t.Errorf("PrivateID.Compare(%s, %s) = %d, bytes.Compare = %d", aStr, bStr, got, bytes.Compare(privA[:], privB[:])) + } + if got := pubA.Compare(pubB); got != want { + t.Errorf("PublicID(%s).Compare(%s) = %d, want %d", aStr, bStr, got, want) + } else if got != bytes.Compare(pubA[:], pubB[:]) { + t.Errorf("PublicID.Compare(%s, %s) = %d, bytes.Compare = %d", aStr, bStr, got, bytes.Compare(pubA[:], pubB[:])) + } + if got, less := privA.Less(privB), want < 0; got != less { + t.Errorf("PrivateID(%s).Less(%s) = %v, want %v", aStr, bStr, got, less) + } + if got, less := pubA.Less(pubB), want < 0; got != less { + t.Errorf("PublicID(%s).Less(%s) = %v, want %v", aStr, bStr, got, less) + } + } + } + + // slices.Compare and bytes.Compare agree on every []byte: unsigned + // lexicographic order, result in {-1, 0, +1}, nil equal to empty, and a + // shorter common prefix less than a longer one. ID slices are always 32 + // bytes; the other cases guard the functions themselves. + check := func(a, b []byte) { + t.Helper() + if got, want := bytes.Compare(a, b), slices.Compare(a, b); got != want { + t.Fatalf("bytes.Compare(%#v, %#v) = %d, slices.Compare = %d", a, b, got, want) + } + } + check(nil, nil) + check(nil, []byte{}) + check([]byte{}, nil) + check(nil, []byte{0}) + check([]byte{0}, nil) + check([]byte{}, []byte{0}) + check([]byte{0}, []byte{}) + for a := range 256 { + for b := range 256 { + check([]byte{byte(a)}, []byte{byte(b)}) + } + } + for i := range 32 { + a := make([]byte, 32) + b := make([]byte, 32) + for _, pair := range [][2]byte{{0, 1}, {0x7f, 0x80}, {0xfe, 0xff}} { + a[i], b[i] = pair[0], pair[1] + check(a, b) + check(b, a) + check(a, append([]byte(nil), a...)) + check(a[:i], a[:i+1]) + check(a[:i+1], a[:i]) + check(a[:i], b[:i]) + a[i], b[i] = 0, 0 + } + } +} + func TestAdd(t *testing.T) { tests := []struct { in string