time: optimize Parse for []byte arguments

When one has a []byte on hand, but desires to call the Parse function,
the conversion from []byte to string would allocate.
This occurs frequently through UnmarshalText and UnmarshalJSON.

This changes it such that the input string never escapes from
any of the Parse functions. Together with the compiler optimization
where the compiler stack allocates any string smaller than 32B
this makes most valid inputs for Parse(layout, string(input))
not require an allocation for the input string.

This optimization works well for most RFC3339 timestamps.
All timestamps with second resolution
(e.g., 2000-01-01T00:00:00Z or 2000-01-01T00:00:00+23:59)
or timestamps with nanosecond resolution in UTC
(e.g., 2000-01-01T00:00:00.123456789Z)
are less than 32B and benefit from this optimization.
Unfortunately, nanosecond timestamps with non-UTC timezones
(e.g., 2000-01-01T00:00:00.123456789+23:59)
do not benefit since they are 35B long.

Previously, this was not possible since the input leaked
to the error and calls to FixedZone with the zone name,
which causes the prover to give up and heap copy the []byte.
We fix this by copying the input string in both cases.
The advantage of this change is that you can now call Parse
with a []byte without allocating (most of the times).
The detriment is that the timezone and error path has an extra allocation.
Handling of timezones were already expensive (3 allocations and 160B allocated),
so the additional cost of another string allocation is relatively minor.
We should optimize for the common case, rather than the exceptional case.

Performance:

    name                  old time/op  new time/op  delta
    ParseRFC3339UTCBytes  54.4ns ± 1%  40.3ns ± 1%  -25.91%  (p=0.000 n=9+10)

Now that parsing of RFC3339 has been heavily optimized in CL 425197,
the performance gains by this optimization becomes relatively more notable.

Related to CL 345488.

Change-Id: I2a8a9cd6354b3bd46c2f57818ed2646a2e485f36
Reviewed-on: https://go-review.googlesource.com/c/go/+/429862
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/time/format.go b/src/time/format.go
index 8051687068bbdd35d2e8ff4f9289892ab0c9045d..485563ede4a2fbb64b60aca4e7a99247c4c6ea26 100644 (file)
--- a/src/time/format.go
+++ b/src/time/format.go
@@ -808,6 +808,20 @@ type ParseError struct {
        Message    string
 }
 
+// newParseError creates a new ParseError.
+// The provided value and valueElem are cloned to avoid escaping their values.
+func newParseError(layout, value, layoutElem, valueElem, message string) *ParseError {
+       valueCopy := cloneString(value)
+       valueElemCopy := cloneString(valueElem)
+       return &ParseError{layout, valueCopy, layoutElem, valueElemCopy, message}
+}
+
+// cloneString returns a string copy of s.
+// Do not use strings.Clone to avoid dependency on strings package.
+func cloneString(s string) string {
+       return string([]byte(s))
+}
+
 // These are borrowed from unicode/utf8 and strconv and replicate behavior in
 // that package, since we can't take a dependency on either.
 const (
@@ -1027,11 +1041,11 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
                stdstr := layout[len(prefix) : len(layout)-len(suffix)]
                value, err = skip(value, prefix)
                if err != nil {
-                       return Time{}, &ParseError{alayout, avalue, prefix, value, ""}
+                       return Time{}, newParseError(alayout, avalue, prefix, value, "")
                }
                if std == 0 {
                        if len(value) != 0 {
-                               return Time{}, &ParseError{alayout, avalue, "", value, ": extra text: " + quote(value)}
+                               return Time{}, newParseError(alayout, avalue, "", value, ": extra text: "+quote(value))
                        }
                        break
                }
@@ -1262,10 +1276,10 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
                        value = value[1+i:]
                }
                if rangeErrString != "" {
-                       return Time{}, &ParseError{alayout, avalue, stdstr, value, ": " + rangeErrString + " out of range"}
+                       return Time{}, newParseError(alayout, avalue, stdstr, value, ": "+rangeErrString+" out of range")
                }
                if err != nil {
-                       return Time{}, &ParseError{alayout, avalue, stdstr, value, ""}
+                       return Time{}, newParseError(alayout, avalue, stdstr, value, "")
                }
        }
        if pmSet && hour < 12 {
@@ -1287,7 +1301,7 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
                        }
                }
                if yday < 1 || yday > 365 {
-                       return Time{}, &ParseError{alayout, avalue, "", value, ": day-of-year out of range"}
+                       return Time{}, newParseError(alayout, avalue, "", value, ": day-of-year out of range")
                }
                if m == 0 {
                        m = (yday-1)/31 + 1
@@ -1299,11 +1313,11 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
                // If month, day already seen, yday's m, d must match.
                // Otherwise, set them from m, d.
                if month >= 0 && month != m {
-                       return Time{}, &ParseError{alayout, avalue, "", value, ": day-of-year does not match month"}
+                       return Time{}, newParseError(alayout, avalue, "", value, ": day-of-year does not match month")
                }
                month = m
                if day >= 0 && day != d {
-                       return Time{}, &ParseError{alayout, avalue, "", value, ": day-of-year does not match day"}
+                       return Time{}, newParseError(alayout, avalue, "", value, ": day-of-year does not match day")
                }
                day = d
        } else {
@@ -1317,7 +1331,7 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
 
        // Validate the day of the month.
        if day < 1 || day > daysIn(Month(month), year) {
-               return Time{}, &ParseError{alayout, avalue, "", value, ": day out of range"}
+               return Time{}, newParseError(alayout, avalue, "", value, ": day out of range")
        }
 
        if z != nil {
@@ -1337,7 +1351,8 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
                }
 
                // Otherwise create fake zone to record offset.
-               t.setLoc(FixedZone(zoneName, zoneOffset))
+               zoneNameCopy := cloneString(zoneName) // avoid leaking the input value
+               t.setLoc(FixedZone(zoneNameCopy, zoneOffset))
                return t, nil
        }
 
@@ -1357,7 +1372,8 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
                        offset, _ = atoi(zoneName[3:]) // Guaranteed OK by parseGMT.
                        offset *= 3600
                }
-               t.setLoc(FixedZone(zoneName, offset))
+               zoneNameCopy := cloneString(zoneName) // avoid leaking the input value
+               t.setLoc(FixedZone(zoneNameCopy, offset))
                return t, nil
        }
 

diff --git a/src/time/time_test.go b/src/time/time_test.go
index 17ce6b3b0b177607a6b866f28b183197ea5cad88..6d9a8fae2f57fd2c15815718086a07bcb773c5ee 100644 (file)
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -1446,15 +1446,35 @@ func BenchmarkParse(b *testing.B) {
        }
 }
 
+const testdataRFC3339UTC = "2020-08-22T11:27:43.123456789Z"
+
 func BenchmarkParseRFC3339UTC(b *testing.B) {
        for i := 0; i < b.N; i++ {
-               Parse(RFC3339, "2020-08-22T11:27:43.123456789Z")
+               Parse(RFC3339, testdataRFC3339UTC)
+       }
+}
+
+var testdataRFC3339UTCBytes = []byte(testdataRFC3339UTC)
+
+func BenchmarkParseRFC3339UTCBytes(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               Parse(RFC3339, string(testdataRFC3339UTCBytes))
        }
 }
 
+const testdataRFC3339TZ = "2020-08-22T11:27:43.123456789-02:00"
+
 func BenchmarkParseRFC3339TZ(b *testing.B) {
        for i := 0; i < b.N; i++ {
-               Parse(RFC3339, "2020-08-22T11:27:43.123456789-02:00")
+               Parse(RFC3339, testdataRFC3339TZ)
+       }
+}
+
+var testdataRFC3339TZBytes = []byte(testdataRFC3339TZ)
+
+func BenchmarkParseRFC3339TZBytes(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               Parse(RFC3339, string(testdataRFC3339TZBytes))
        }
 }
 
@@ -1561,6 +1581,16 @@ func TestMarshalBinaryVersion2(t *testing.T) {
        }
 }
 
+func TestUnmarshalTextAllocations(t *testing.T) {
+       in := []byte(testdataRFC3339UTC) // short enough to be stack allocated
+       if allocs := testing.AllocsPerRun(100, func() {
+               var t Time
+               t.UnmarshalText(in)
+       }); allocs != 0 {
+               t.Errorf("got %v allocs, want 0 allocs", allocs)
+       }
+}
+
 // Issue 17720: Zero value of time.Month fails to print
 func TestZeroMonthString(t *testing.T) {
        if got, want := Month(0).String(), "%!Month(0)"; got != want {