Browse Source

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 <[email protected]>
Joe Tsai 3 years ago
parent
commit
7e6c5a2db4
2 changed files with 8 additions and 306 deletions
  1. 8 127
      tstime/tstime.go
  2. 0 179
      tstime/tstime_test.go

+ 8 - 127
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 {

+ 0 - 179
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)
-	}
-}