From aa9d7f466550b567e11c317f64ea5eac76d31314 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Wed, 18 Nov 2020 11:59:02 -0800 Subject: [PATCH] tstime: add Parse3339B, for byte slices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use go4.org/mem for memory safety. A slight performance hit, but a huge performance win for clients who start with a []byte. The perf hit is due largely to the MapHash call, which adds ~25ns. That is necessary to keep the fast path allocation-free. name old time/op new time/op delta GoParse3339/Z-8 281ns ± 1% 283ns ± 2% ~ (p=0.366 n=9+9) GoParse3339/TZ-8 509ns ± 0% 510ns ± 1% ~ (p=0.059 n=9+9) GoParse3339InLocation-8 330ns ± 1% 330ns ± 0% ~ (p=0.802 n=10+6) Parse3339/Z-8 69.3ns ± 1% 74.4ns ± 1% +7.45% (p=0.000 n=9+10) Parse3339/TZ-8 110ns ± 1% 140ns ± 3% +27.42% (p=0.000 n=9+10) ParseInt-8 8.20ns ± 1% 8.17ns ± 1% ~ (p=0.452 n=9+9) name old alloc/op new alloc/op delta GoParse3339/Z-8 0.00B 0.00B ~ (all equal) GoParse3339/TZ-8 160B ± 0% 160B ± 0% ~ (all equal) GoParse3339InLocation-8 0.00B 0.00B ~ (all equal) Parse3339/Z-8 0.00B 0.00B ~ (all equal) Parse3339/TZ-8 0.00B 0.00B ~ (all equal) name old allocs/op new allocs/op delta GoParse3339/Z-8 0.00 0.00 ~ (all equal) GoParse3339/TZ-8 3.00 ± 0% 3.00 ± 0% ~ (all equal) GoParse3339InLocation-8 0.00 0.00 ~ (all equal) Parse3339/Z-8 0.00 0.00 ~ (all equal) Parse3339/TZ-8 0.00 0.00 ~ (all equal) Signed-off-by: Josh Bleecher Snyder --- cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 1 + go.mod | 2 +- go.sum | 2 + tstime/tstime.go | 119 +++++++++++++++++++++--------------- tstime/tstime_test.go | 10 +-- 6 files changed, 81 insertions(+), 54 deletions(-) diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index ff4685576..5c5071ca2 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -170,6 +170,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep hash/adler32 from compress/zlib hash/crc32 from compress/gzip+ hash/fnv from tailscale.com/wgengine/magicsock + hash/maphash from go4.org/mem html from tailscale.com/ipn/ipnstate io from bufio+ io/ioutil from crypto/tls+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index bae586af8..15439e0c6 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -181,6 +181,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de hash/adler32 from compress/zlib hash/crc32 from compress/gzip+ hash/fnv from tailscale.com/wgengine/magicsock + hash/maphash from go4.org/mem html from html/template+ html/template from net/http/pprof io from bufio+ diff --git a/go.mod b/go.mod index cbba5fa7e..872604897 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/tailscale/wireguard-go v0.0.0-20201021041318-a6168fd06b3f github.com/tcnksm/go-httpstat v0.2.0 github.com/toqueteos/webbrowser v1.2.0 - go4.org/mem v0.0.0-20200706164138-185c595c3ecc + go4.org/mem v0.0.0-20201119185036-c04c5a6ff174 golang.org/x/crypto v0.0.0-20201112155050-0c6587e931a9 golang.org/x/net v0.0.0-20201110031124-69a78807bb2b golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d diff --git a/go.sum b/go.sum index 516b04585..2e2723d18 100644 --- a/go.sum +++ b/go.sum @@ -125,6 +125,8 @@ github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8/go.mod h1:HUYIGzjTL3rfEspMx github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go4.org/mem v0.0.0-20200706164138-185c595c3ecc h1:paujszgN6SpsO/UsXC7xax3gQAKz/XQKCYZLQdU34Tw= go4.org/mem v0.0.0-20200706164138-185c595c3ecc/go.mod h1:NEYvpHWemiG/E5UWfaN5QAIGZeT1sa0Z2UNk6oeMb/k= +go4.org/mem v0.0.0-20201119185036-c04c5a6ff174 h1:vSug/WNOi2+4jrKdivxayTN/zd8EA1UrStjpWvvo1jk= +go4.org/mem v0.0.0-20201119185036-c04c5a6ff174/go.mod h1:reUoABIJ9ikfM5sgtSF3Wushcza7+WeD01VB9Lirh3g= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191002192127-34f69633bfdc/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= diff --git a/tstime/tstime.go b/tstime/tstime.go index ff515d535..61aab5ca4 100644 --- a/tstime/tstime.go +++ b/tstime/tstime.go @@ -8,109 +8,119 @@ package tstime import ( "errors" "fmt" - "strings" "sync" "time" + + "go4.org/mem" ) +var memZ = mem.S("Z") + // zoneOf returns the RFC3339 zone suffix (either "Z" or like // "+08:30"), or the empty string if it's invalid or not something we // want to cache. -func zoneOf(s string) string { - if strings.HasSuffix(s, "Z") { - return "Z" +func zoneOf(s mem.RO) mem.RO { + if mem.HasSuffix(s, memZ) { + return memZ } - if len(s) < len("2020-04-05T15:56:00+08:00") { + if s.Len() < len("2020-04-05T15:56:00+08:00") { // Too short, invalid? Let time.Parse fail on it. - return "" + return mem.S("") } - zone := s[len(s)-len("+08:00"):] - if c := zone[0]; c == '+' || c == '-' { - min := zone[len("+08:"):] - switch min { - case "00", "15", "30": + zone := s.SliceFrom(s.Len() - len("+08:00")) + if c := zone.At(0); c == '+' || c == '-' { + min := zone.SliceFrom(len("+08:")) + if min.EqualString("00") || min.EqualString("15") || min.EqualString("30") { return zone } } - return "" + return mem.S("") } -// locCache maps from zone offset suffix string ("+08:00") => -// *time.Location (from FixedLocation). +// locCache maps from hash of zone offset suffix string ("+08:00") => +// {zone string, *time.Location (from FixedLocation)}. var locCache sync.Map -func getLocation(zone, timeValue string) (*time.Location, error) { - if zone == "Z" { +type locCacheEntry struct { + zone string + loc *time.Location +} + +func getLocation(zone, timeValue mem.RO) (*time.Location, error) { + if zone.EqualString("Z") { return time.UTC, nil } - if loci, ok := locCache.Load(zone); ok { - return loci.(*time.Location), nil + key := zone.MapHash() + if entry, ok := locCache.Load(key); ok { + // We're keying only on a hash; double-check zone to ensure no spurious collisions. + e := entry.(locCacheEntry) + if zone.EqualString(e.zone) { + return e.loc, nil + } } // TODO(bradfitz): just parse it and call time.FixedLocation. // For now, just have time.Parse do it once: - t, err := time.Parse(time.RFC3339Nano, timeValue) + t, err := time.Parse(time.RFC3339Nano, timeValue.StringCopy()) if err != nil { return nil, err } loc := t.Location() - locCache.LoadOrStore(zone, loc) + locCache.LoadOrStore(key, locCacheEntry{zone: zone.StringCopy(), loc: loc}) return loc, nil - } -// Parse3339 is a wrapper around time.Parse(time.RFC3339Nano, s) that caches -// timezone Locations for future parses. -func Parse3339(s string) (time.Time, error) { +func parse3339m(s mem.RO) (time.Time, error) { zone := zoneOf(s) - if zone == "" { + if zone.Len() == 0 { // Invalid or weird timezone offset. Use slow path, // which'll probably return an error. - return time.Parse(time.RFC3339Nano, s) + return time.Parse(time.RFC3339Nano, s.StringCopy()) } loc, err := getLocation(zone, s) if err != nil { return time.Time{}, err } - s = s[:len(s)-len(zone)] // remove zone suffix + s = s.SliceTo(s.Len() - zone.Len()) // remove zone suffix var year, mon, day, hr, min, sec, nsec int const baseLen = len("2020-04-05T15:56:00") - if len(s) < baseLen || - !parseInt(s[:4], &year) || - s[4] != '-' || - !parseInt(s[5:7], &mon) || - s[7] != '-' || - !parseInt(s[8:10], &day) || - s[10] != 'T' || - !parseInt(s[11:13], &hr) || - s[13] != ':' || - !parseInt(s[14:16], &min) || - s[16] != ':' || - !parseInt(s[17:19], &sec) { + if s.Len() < baseLen || + !parseInt(s.SliceTo(4), &year) || + s.At(4) != '-' || + !parseInt(s.Slice(5, 7), &mon) || + s.At(7) != '-' || + !parseInt(s.Slice(8, 10), &day) || + s.At(10) != 'T' || + !parseInt(s.Slice(11, 13), &hr) || + s.At(13) != ':' || + !parseInt(s.Slice(14, 16), &min) || + s.At(16) != ':' || + !parseInt(s.Slice(17, 19), &sec) { return time.Time{}, errors.New("invalid time") } - nsStr := s[baseLen:] - if nsStr != "" { - if nsStr[0] != '.' { + nsStr := s.SliceFrom(baseLen) + if nsStr.Len() != 0 { + if nsStr.At(0) != '.' { return time.Time{}, errors.New("invalid optional nanosecond prefix") } - if !parseInt(nsStr[1:], &nsec) { - return time.Time{}, fmt.Errorf("invalid optional nanosecond number %q", nsStr[1:]) + nsStr = nsStr.SliceFrom(1) + if !parseInt(nsStr, &nsec) { + return time.Time{}, fmt.Errorf("invalid optional nanosecond number %q", nsStr.StringCopy()) } - for i := 0; i < len("999999999")-(len(nsStr)-1); i++ { + for i := 0; i < len("999999999")-nsStr.Len(); i++ { nsec *= 10 } } return time.Date(year, time.Month(mon), day, hr, min, sec, nsec, loc), nil } -func parseInt(s string, dst *int) bool { - if len(s) == 0 || len(s) > len("999999999") { +func parseInt(s mem.RO, dst *int) bool { + if s.Len() == 0 || s.Len() > len("999999999") { *dst = 0 return false } n := 0 - for i := 0; i < len(s); i++ { - d := s[i] - '0' + for i := 0; i < s.Len(); i++ { + d := s.At(i) - '0' if d > 9 { *dst = 0 return false @@ -120,3 +130,14 @@ func parseInt(s string, dst *int) bool { *dst = n return true } + +// Parse3339 is a wrapper around time.Parse(time.RFC3339Nano, s) that caches +// timezone Locations for future parses. +func Parse3339(s string) (time.Time, error) { + return parse3339m(mem.S(s)) +} + +// Parse3339B is Parse3339 but for byte slices. +func Parse3339B(b []byte) (time.Time, error) { + return parse3339m(mem.B(b)) +} diff --git a/tstime/tstime_test.go b/tstime/tstime_test.go index 7f46576e8..8e8a27743 100644 --- a/tstime/tstime_test.go +++ b/tstime/tstime_test.go @@ -7,6 +7,8 @@ package tstime import ( "testing" "time" + + "go4.org/mem" ) func TestParse3339(t *testing.T) { @@ -70,8 +72,8 @@ func TestZoneOf(t *testing.T) { {"+08:00", ""}, // too short } for _, tt := range tests { - if got := zoneOf(tt.in); got != tt.want { - t.Errorf("zoneOf(%q) = %q; want %q", tt.in, got, tt.want) + if got := zoneOf(mem.S(tt.in)); !got.EqualString(tt.want) { + t.Errorf("zoneOf(%q) = %q; want %q", tt.in, got.StringCopy(), tt.want) } } } @@ -93,7 +95,7 @@ func TestParseInt(t *testing.T) { for _, tt := range tests { var got int - gotRet := parseInt(tt.in, &got) + gotRet := parseInt(mem.S(tt.in), &got) if gotRet != tt.ret || got != tt.want { t.Errorf("parseInt(%q) = %v, %d; want %v, %d", tt.in, gotRet, got, tt.ret, tt.want) } @@ -182,6 +184,6 @@ func BenchmarkParse3339(b *testing.B) { func BenchmarkParseInt(b *testing.B) { var out int for i := 0; i < b.N; i++ { - parseInt("148487491", &out) + parseInt(mem.S("148487491"), &out) } }