From 7e6c5a2db46f874dac8ef9e8cdc9916fd7592a1a Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 6 Mar 2023 18:05:51 -0800 Subject: [PATCH] tstime: rely on stdlib parse functionality (#7482) The time.Parse function has been optimized to the point where it is faster than our custom implementation. See upstream changes in: * https://go.dev/cl/429862 * https://go.dev/cl/425197 * https://go.dev/cl/425116 Performance: BenchmarkGoParse3339/Z 38.75 ns/op 0 B/op 0 allocs/op BenchmarkGoParse3339/TZ 54.02 ns/op 0 B/op 0 allocs/op BenchmarkParse3339/Z 40.17 ns/op 0 B/op 0 allocs/op BenchmarkParse3339/TZ 87.06 ns/op 0 B/op 0 allocs/op We can see that the stdlib implementation is now faster. Signed-off-by: Joe Tsai --- tstime/tstime.go | 135 ++----------------------------- tstime/tstime_test.go | 179 ------------------------------------------ 2 files changed, 8 insertions(+), 306 deletions(-) diff --git a/tstime/tstime.go b/tstime/tstime.go index cb3b467e6..cc76755cc 100644 --- a/tstime/tstime.go +++ b/tstime/tstime.go @@ -6,145 +6,26 @@ package tstime import ( "context" - "errors" - "fmt" "strconv" "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 mem.RO) mem.RO { - if mem.HasSuffix(s, memZ) { - return memZ - } - if s.Len() < len("2020-04-05T15:56:00+08:00") { - // Too short, invalid? Let time.Parse fail on it. - return mem.S("") - } - 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 mem.S("") -} - -// locCache maps from hash of zone offset suffix string ("+08:00") => -// {zone string, *time.Location (from FixedLocation)}. -var locCache sync.Map - -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 - } - 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.StringCopy()) - if err != nil { - return nil, err - } - loc := t.Location() - locCache.LoadOrStore(key, locCacheEntry{zone: zone.StringCopy(), loc: loc}) - return loc, nil -} - -func parse3339m(s mem.RO) (time.Time, error) { - zone := zoneOf(s) - if zone.Len() == 0 { - // Invalid or weird timezone offset. Use slow path, - // which'll probably return an error. - return time.Parse(time.RFC3339Nano, s.StringCopy()) - } - loc, err := getLocation(zone, s) - if err != nil { - return time.Time{}, err - } - 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 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.SliceFrom(baseLen) - if nsStr.Len() != 0 { - if nsStr.At(0) != '.' { - return time.Time{}, errors.New("invalid optional nanosecond prefix") - } - 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")-nsStr.Len(); i++ { - nsec *= 10 - } - } - return time.Date(year, time.Month(mon), day, hr, min, sec, nsec, loc), nil -} - -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 < s.Len(); i++ { - d := s.At(i) - '0' - if d > 9 { - *dst = 0 - return false - } - n = n*10 + int(d) - } - *dst = n - return true -} - -// Parse3339 is a wrapper around time.Parse(time.RFC3339Nano, s) that caches -// timezone Locations for future parses. +// Parse3339 is a wrapper around time.Parse(time.RFC3339, s). func Parse3339(s string) (time.Time, error) { - return parse3339m(mem.S(s)) + return time.Parse(time.RFC3339, s) } // Parse3339B is Parse3339 but for byte slices. func Parse3339B(b []byte) (time.Time, error) { - return parse3339m(mem.B(b)) + var t time.Time + if err := t.UnmarshalText(b); err != nil { + return Parse3339(string(b)) // reproduce same error message + } + return t, nil } -// ParseDuration is more expressive than time.ParseDuration, +// ParseDuration is more expressive than [time.ParseDuration], // also accepting 'd' (days) and 'w' (weeks) literals. func ParseDuration(s string) (time.Duration, error) { for { diff --git a/tstime/tstime_test.go b/tstime/tstime_test.go index b44f9bc5a..3ffeaf0ff 100644 --- a/tstime/tstime_test.go +++ b/tstime/tstime_test.go @@ -6,101 +6,8 @@ package tstime import ( "testing" "time" - - "go4.org/mem" ) -func TestParse3339(t *testing.T) { - tests := []string{ - "2020-04-05T15:56:00Z", - "2020-04-05T15:56:00.1234Z", - "2020-04-05T15:56:00+08:00", - - "2020-04-05T15:56:00.1+08:00", - "2020-04-05T15:56:00.12+08:00", - "2020-04-05T15:56:00.012+08:00", - "2020-04-05T15:56:00.0012+08:00", - "2020-04-05T15:56:00.148487491+08:00", - - "2020x04-05T15:56:00.1234+08:00", - "2020-04x05T15:56:00.1234+08:00", - "2020-04-05x15:56:00.1234+08:00", - "2020-04-05T15x56:00.1234+08:00", - "2020-04-05T15:56x00.1234+08:00", - "2020-04-05T15:56:00x1234+08:00", - } - for _, s := range tests { - t.Run(s, func(t *testing.T) { - goTime, goErr := time.Parse(time.RFC3339Nano, s) - - Parse3339(s) // prime the tz cache so next parse use fast path - got, err := Parse3339(s) - - if (err == nil) != (goErr == nil) { - t.Fatalf("for %q, go err = %v; our err = %v", s, goErr, err) - } - if err != nil { - return - } - if !goTime.Equal(got) { - t.Errorf("for %q, times not Equal: we got %v, want go's %v", s, got, goTime) - } - if goStr, ourStr := goTime.Format(time.RFC3339Nano), got.Format(time.RFC3339Nano); goStr != ourStr { - t.Errorf("for %q, strings not equal: we got %q, want go's %q", s, ourStr, goStr) - } - - }) - } - -} - -func TestZoneOf(t *testing.T) { - tests := []struct { - in, want string - }{ - {"2020-04-05T15:56:00+08:00", "+08:00"}, - {"2020-04-05T15:56:00-08:00", "-08:00"}, - {"2020-04-05T15:56:00.12345-08:00", "-08:00"}, - {"2020-04-05T15:56:00.12345-08:00", "-08:00"}, - {"2020-04-05T15:56:00.12345-08:30", "-08:30"}, - {"2020-04-05T15:56:00.12345-08:15", "-08:15"}, - {"2020-04-05T15:56:00.12345-08:17", ""}, // don't cache weird offsets - {"2020-04-05T15:56:00.12345Z", "Z"}, - {"2020-04-05T15:56:00Z", "Z"}, - {"123+08:00", ""}, // too short - {"+08:00", ""}, // too short - } - for _, tt := range tests { - if got := zoneOf(mem.S(tt.in)); !got.EqualString(tt.want) { - t.Errorf("zoneOf(%q) = %q; want %q", tt.in, got.StringCopy(), tt.want) - } - } -} - -func TestParseInt(t *testing.T) { - tests := []struct { - in string - want int - ret bool - }{ - {"", 0, false}, - {"1", 1, true}, - {"999999999", 999999999, true}, - {"123456789", 123456789, true}, - {"000000", 0, true}, - {"bork", 0, false}, - {"123bork", 0, false}, - } - - for _, tt := range tests { - var got int - 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) - } - } -} - func TestParseDuration(t *testing.T) { tests := []struct { in string @@ -127,89 +34,3 @@ func TestParseDuration(t *testing.T) { } } } - -func BenchmarkGoParse3339(b *testing.B) { - run := func(in string) func(*testing.B) { - return func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - _, err := time.Parse(time.RFC3339Nano, in) - if err != nil { - b.Fatal(err) - } - } - } - } - b.Run("Z", run("2020-04-05T15:56:00.148487491Z")) - b.Run("TZ", run("2020-04-05T15:56:00.148487491+08:00")) -} - -func BenchmarkGoParse3339InLocation(b *testing.B) { - b.ReportAllocs() - const in = `2020-04-05T15:56:00.148487491+08:00` - - t, err := time.Parse(time.RFC3339Nano, in) - if err != nil { - b.Fatal(err) - } - loc := t.Location() - - t2, err := time.ParseInLocation(time.RFC3339Nano, in, loc) - if err != nil { - b.Fatal(err) - } - if !t.Equal(t2) { - b.Fatal("not equal") - } - if s1, s2 := t.Format(time.RFC3339Nano), t2.Format(time.RFC3339Nano); s1 != s2 { - b.Fatalf("times don't stringify the same: %q vs %q", s1, s2) - } - - b.ResetTimer() - for i := 0; i < b.N; i++ { - _, err := time.ParseInLocation(time.RFC3339Nano, in, loc) - if err != nil { - b.Fatal(err) - } - } -} - -func BenchmarkParse3339(b *testing.B) { - run := func(in string) func(*testing.B) { - return func(b *testing.B) { - b.ReportAllocs() - t, err := time.Parse(time.RFC3339Nano, in) - if err != nil { - b.Fatal(err) - } - - t2, err := Parse3339(in) - if err != nil { - b.Fatal(err) - } - if !t.Equal(t2) { - b.Fatal("not equal") - } - if s1, s2 := t.Format(time.RFC3339Nano), t2.Format(time.RFC3339Nano); s1 != s2 { - b.Fatalf("times don't stringify the same: %q vs %q", s1, s2) - } - - for i := 0; i < b.N; i++ { - _, err := Parse3339(in) - if err != nil { - b.Fatal(err) - } - } - } - } - b.Run("Z", run("2020-04-05T15:56:00.148487491Z")) - b.Run("TZ", run("2020-04-05T15:56:00.148487491+08:00")) -} - -func BenchmarkParseInt(b *testing.B) { - b.ReportAllocs() - var out int - for i := 0; i < b.N; i++ { - parseInt(mem.S("148487491"), &out) - } -}