From c6fadd6d71ac884e74bcedd3942576c397108093 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 1 Sep 2023 18:15:19 -0700 Subject: [PATCH] all: implement AppendText alongside MarshalText (#9207) This eventually allows encoding packages that may respect the proposed encoding.TextAppender interface. The performance gains from this is between 10-30%. Updates tailscale/corp#14379 Signed-off-by: Joe Tsai --- control/controlclient/status.go | 4 ++++ tailcfg/tailcfg.go | 21 ++++++++------------- tailcfg/tailcfg_export_test.go | 6 ------ tailcfg/tailcfg_test.go | 25 ------------------------- tka/aum.go | 18 +++++++++++++++--- types/key/chal.go | 7 ++++++- types/key/disco.go | 7 ++++++- types/key/machine.go | 14 ++++++++++++-- types/key/nl.go | 16 +++++++++++++--- types/key/node.go | 14 ++++++++++++-- types/key/util.go | 19 ++++++++++++++----- types/logid/id.go | 26 ++++++++++++++++++-------- 12 files changed, 108 insertions(+), 69 deletions(-) delete mode 100644 tailcfg/tailcfg_export_test.go diff --git a/control/controlclient/status.go b/control/controlclient/status.go index 51b527a40..d0fdf80d7 100644 --- a/control/controlclient/status.go +++ b/control/controlclient/status.go @@ -37,6 +37,10 @@ const ( StateSynchronized // connected and received map update ) +func (s State) AppendText(b []byte) ([]byte, error) { + return append(b, s.String()...), nil +} + func (s State) MarshalText() ([]byte, error) { return []byte(s.String()), nil } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 754d90eda..501a6bf62 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -7,7 +7,6 @@ package tailcfg import ( "bytes" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -445,6 +444,10 @@ const ( MachineInvalid // server has explicitly rejected this machine key ) +func (m MachineStatus) AppendText(b []byte) ([]byte, error) { + return append(b, m.String()...), nil +} + func (m MachineStatus) MarshalText() ([]byte, error) { return []byte(m.String()), nil } @@ -921,6 +924,10 @@ const ( SignatureV2 ) +func (st SignatureType) AppendText(b []byte) ([]byte, error) { + return append(b, st.String()...), nil +} + func (st SignatureType) MarshalText() ([]byte, error) { return []byte(st.String()), nil } @@ -1765,18 +1772,6 @@ type Debug struct { Exit *int `json:",omitempty"` } -func appendKey(base []byte, prefix string, k [32]byte) []byte { - ret := append(base, make([]byte, len(prefix)+64)...) - buf := ret[len(base):] - copy(buf, prefix) - hex.Encode(buf[len(prefix):], k[:]) - return ret -} - -func keyMarshalText(prefix string, k [32]byte) []byte { - return appendKey(nil, prefix, k) -} - func (id ID) String() string { return fmt.Sprintf("id:%x", int64(id)) } func (id UserID) String() string { return fmt.Sprintf("userid:%x", int64(id)) } func (id LoginID) String() string { return fmt.Sprintf("loginid:%x", int64(id)) } diff --git a/tailcfg/tailcfg_export_test.go b/tailcfg/tailcfg_export_test.go deleted file mode 100644 index 11d68fa84..000000000 --- a/tailcfg/tailcfg_export_test.go +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package tailcfg - -var ExportKeyMarshalText = keyMarshalText diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 77598982a..d18c01209 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -16,11 +16,9 @@ import ( "time" . "tailscale.com/tailcfg" - "tailscale.com/tstest" "tailscale.com/types/key" "tailscale.com/types/ptr" "tailscale.com/util/must" - "tailscale.com/version" ) func fieldsOf(t reflect.Type) (fields []string) { @@ -683,29 +681,6 @@ func TestEndpointTypeMarshal(t *testing.T) { } } -var sinkBytes []byte - -func BenchmarkKeyMarshalText(b *testing.B) { - b.ReportAllocs() - var k [32]byte - for i := 0; i < b.N; i++ { - sinkBytes = ExportKeyMarshalText("prefix", k) - } -} - -func TestAppendKeyAllocs(t *testing.T) { - if version.IsRace() { - t.Skip("skipping in race detector") // append(b, make([]byte, N)...) not optimized in compiler with race - } - var k [32]byte - err := tstest.MinAllocsPerRun(t, 1, func() { - sinkBytes = ExportKeyMarshalText("prefix", k) - }) - if err != nil { - t.Fatal(err) - } -} - func TestRegisterRequestNilClone(t *testing.T) { var nilReq *RegisterRequest got := nilReq.Clone() diff --git a/tka/aum.go b/tka/aum.go index d21ca199b..a4e2ff55f 100644 --- a/tka/aum.go +++ b/tka/aum.go @@ -9,6 +9,7 @@ import ( "encoding/base32" "errors" "fmt" + "slices" "github.com/fxamacker/cbor/v2" "golang.org/x/crypto/blake2s" @@ -37,11 +38,22 @@ func (h *AUMHash) UnmarshalText(text []byte) error { return nil } +// TODO(https://go.dev/issue/53693): Use base32.Encoding.AppendEncode instead. +func base32AppendEncode(enc *base32.Encoding, dst, src []byte) []byte { + n := enc.EncodedLen(len(src)) + dst = slices.Grow(dst, n) + enc.Encode(dst[len(dst):][:n], src) + return dst[:len(dst)+n] +} + +// AppendText implements encoding.TextAppender. +func (h AUMHash) AppendText(b []byte) ([]byte, error) { + return base32AppendEncode(base32StdNoPad, b, h[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (h AUMHash) MarshalText() ([]byte, error) { - b := make([]byte, base32StdNoPad.EncodedLen(len(h))) - base32StdNoPad.Encode(b, h[:]) - return b, nil + return h.AppendText(nil) } // IsZero returns true if the hash is the empty value. diff --git a/types/key/chal.go b/types/key/chal.go index 8b46f4e52..742ac5479 100644 --- a/types/key/chal.go +++ b/types/key/chal.go @@ -72,9 +72,14 @@ func (k ChallengePublic) String() string { return string(bs) } +// AppendText implements encoding.TextAppender. +func (k ChallengePublic) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, chalPublicHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k ChallengePublic) MarshalText() ([]byte, error) { - return toHex(k.k[:], chalPublicHexPrefix), nil + return k.AppendText(nil) } // UnmarshalText implements encoding.TextUnmarshaler. diff --git a/types/key/disco.go b/types/key/disco.go index 8f8ee5c88..14005b506 100644 --- a/types/key/disco.go +++ b/types/key/disco.go @@ -127,9 +127,14 @@ func (k DiscoPublic) String() string { return string(bs) } +// AppendText implements encoding.TextAppender. +func (k DiscoPublic) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, discoPublicHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k DiscoPublic) MarshalText() ([]byte, error) { - return toHex(k.k[:], discoPublicHexPrefix), nil + return k.AppendText(nil) } // MarshalText implements encoding.TextUnmarshaler. diff --git a/types/key/machine.go b/types/key/machine.go index 0a81e63a7..a05f3cc1f 100644 --- a/types/key/machine.go +++ b/types/key/machine.go @@ -67,9 +67,14 @@ func (k MachinePrivate) Public() MachinePublic { return ret } +// AppendText implements encoding.TextAppender. +func (k MachinePrivate) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, machinePrivateHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k MachinePrivate) MarshalText() ([]byte, error) { - return toHex(k.k[:], machinePrivateHexPrefix), nil + return k.AppendText(nil) } // MarshalText implements encoding.TextUnmarshaler. @@ -243,9 +248,14 @@ func (k MachinePublic) String() string { return string(bs) } +// AppendText implements encoding.TextAppender. +func (k MachinePublic) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, machinePublicHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k MachinePublic) MarshalText() ([]byte, error) { - return toHex(k.k[:], machinePublicHexPrefix), nil + return k.AppendText(nil) } // MarshalText implements encoding.TextUnmarshaler. diff --git a/types/key/nl.go b/types/key/nl.go index 1b70e2186..e0b4e5ca6 100644 --- a/types/key/nl.go +++ b/types/key/nl.go @@ -61,9 +61,14 @@ func (k *NLPrivate) UnmarshalText(b []byte) error { return parseHex(k.k[:], mem.B(b), mem.S(nlPrivateHexPrefix)) } +// AppendText implements encoding.TextAppender. +func (k NLPrivate) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, nlPrivateHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k NLPrivate) MarshalText() ([]byte, error) { - return toHex(k.k[:], nlPrivateHexPrefix), nil + return k.AppendText(nil) } // Equal reports whether k and other are the same key. @@ -132,10 +137,15 @@ func (k *NLPublic) UnmarshalText(b []byte) error { return parseHex(k.k[:], mem.B(b), mem.S(nlPublicHexPrefix)) } +// AppendText implements encoding.TextAppender. +func (k NLPublic) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, nlPublicHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler, emitting a // representation of the form nlpub:. func (k NLPublic) MarshalText() ([]byte, error) { - return toHex(k.k[:], nlPublicHexPrefix), nil + return k.AppendText(nil) } // CLIString returns a marshalled representation suitable for use @@ -143,7 +153,7 @@ func (k NLPublic) MarshalText() ([]byte, error) { // the nlpub: form emitted by MarshalText. Both forms can // be decoded by UnmarshalText. func (k NLPublic) CLIString() string { - return string(toHex(k.k[:], nlPublicHexPrefixCLI)) + return string(appendHexKey(nil, nlPublicHexPrefixCLI, k.k[:])) } // Verifier returns a ed25519.PublicKey that can be used to diff --git a/types/key/node.go b/types/key/node.go index a84057231..6e429d2f1 100644 --- a/types/key/node.go +++ b/types/key/node.go @@ -103,9 +103,14 @@ func (k NodePrivate) Public() NodePublic { return ret } +// AppendText implements encoding.TextAppender. +func (k NodePrivate) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, nodePrivateHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k NodePrivate) MarshalText() ([]byte, error) { - return toHex(k.k[:], nodePrivateHexPrefix), nil + return k.AppendText(nil) } // MarshalText implements encoding.TextUnmarshaler. @@ -308,9 +313,14 @@ func (k NodePublic) String() string { return string(bs) } +// AppendText implements encoding.TextAppender. +func (k NodePublic) AppendText(b []byte) ([]byte, error) { + return appendHexKey(b, nodePublicHexPrefix, k.k[:]), nil +} + // MarshalText implements encoding.TextMarshaler. func (k NodePublic) MarshalText() ([]byte, error) { - return toHex(k.k[:], nodePublicHexPrefix), nil + return k.AppendText(nil) } // MarshalText implements encoding.TextUnmarshaler. diff --git a/types/key/util.go b/types/key/util.go index cf079be95..f20cb4279 100644 --- a/types/key/util.go +++ b/types/key/util.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "slices" "go4.org/mem" ) @@ -49,11 +50,19 @@ func clamp25519Private(b []byte) { b[31] = (b[31] & 127) | 64 } -func toHex(k []byte, prefix string) []byte { - ret := make([]byte, len(prefix)+len(k)*2) - copy(ret, prefix) - hex.Encode(ret[len(prefix):], k) - return ret +func appendHexKey(dst []byte, prefix string, key []byte) []byte { + dst = slices.Grow(dst, len(prefix)+hex.EncodedLen(len(key))) + dst = append(dst, prefix...) + dst = hexAppendEncode(dst, key) + return dst +} + +// TODO(https://go.dev/issue/53693): Use hex.AppendEncode instead. +func hexAppendEncode(dst, src []byte) []byte { + n := hex.EncodedLen(len(src)) + dst = slices.Grow(dst, n) + hex.Encode(dst[len(dst):][:n], src) + return dst[:len(dst)+n] } // parseHex decodes a key string of the form "" diff --git a/types/logid/id.go b/types/logid/id.go index 7f620033c..ac4369a4e 100644 --- a/types/logid/id.go +++ b/types/logid/id.go @@ -38,8 +38,12 @@ func ParsePrivateID(in string) (out PrivateID, err error) { return out, err } +func (id PrivateID) AppendText(b []byte) ([]byte, error) { + return hexAppendEncode(b, id[:]), nil +} + func (id PrivateID) MarshalText() ([]byte, error) { - return formatID(id), nil + return id.AppendText(nil) } func (id *PrivateID) UnmarshalText(in []byte) error { @@ -47,7 +51,7 @@ func (id *PrivateID) UnmarshalText(in []byte) error { } func (id PrivateID) String() string { - return string(formatID(id)) + return string(hexAppendEncode(nil, id[:])) } func (id PrivateID) IsZero() bool { @@ -70,8 +74,12 @@ func ParsePublicID(in string) (out PublicID, err error) { return out, err } +func (id PublicID) AppendText(b []byte) ([]byte, error) { + return hexAppendEncode(b, id[:]), nil +} + func (id PublicID) MarshalText() ([]byte, error) { - return formatID(id), nil + return id.AppendText(nil) } func (id *PublicID) UnmarshalText(in []byte) error { @@ -79,7 +87,7 @@ func (id *PublicID) UnmarshalText(in []byte) error { } func (id PublicID) String() string { - return string(formatID(id)) + return string(hexAppendEncode(nil, id[:])) } func (id1 PublicID) Less(id2 PublicID) bool { @@ -98,10 +106,12 @@ func (id PublicID) Prefix64() uint64 { return binary.BigEndian.Uint64(id[:8]) } -func formatID(in [32]byte) []byte { - var hexArr [2 * len(in)]byte - hex.Encode(hexArr[:], in[:]) - return hexArr[:] +// TODO(https://go.dev/issue/53693): Use hex.AppendEncode instead. +func hexAppendEncode(dst, src []byte) []byte { + n := hex.EncodedLen(len(src)) + dst = slices.Grow(dst, n) + hex.Encode(dst[len(dst):][:n], src) + return dst[:len(dst)+n] } func parseID[Bytes []byte | string](funcName string, out *[32]byte, in Bytes) (err error) {