performance improvement for time format

[chanxuehong]

Description

use custom format code instead of standard time.Time.Format

BenchmarkGetTimeDateClockIndependent-8 10504020 114 ns/op 0 B/op 0 allocs/op BenchmarkGetTimeDateClockTogether-8 23327814 48.1 ns/op 0 B/op 0 allocs/op BenchmarkFormatDatetime-8 12794754 72.7 ns/op 0 B/op 0 allocs/op BenchmarkFormatDatetimeViaStandardFormat-8 4033940 303 ns/op 32 B/op 1 allocs/op 

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

[methane]

Please include macro benchmark (benchmark through executing SQL) too for both of setting interpolateParams false and true.
Without it, we can't know this is real improvement.
Especially, this PR seems to increase allocation of interpolateParams.

[chanxuehong]

Please include macro benchmark (benchmark through executing SQL) too for both of setting interpolateParams false and true.
Without it, we can't know this is real improvement.
Especially, this PR seems to increase allocation of interpolateParams.

Sorry, I do n’t know how to run two versions of the mysql driver in one test at the same time, so I did separate tests. The following is my tests:

here is the table

CREATETABLE `test` ( `id`int(11) unsigned NOT NULL AUTO_INCREMENT, `dt` datetime NOT NULL DEFAULT '1000-01-01 00:00:00', PRIMARY KEY (`id`), KEY `idx_dt` (`dt`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8mb4; insert into`test`(`dt`) values(`2020-05-14 12:37:06`);

---

interpolateParams=false

package main import ( "database/sql""fmt""testing""time" _ "github.com/go-sql-driver/mysql" ) varLoc=time.FixedZone("Asia/Shanghai", 8*60*60) varDB*sql.DBfuncinit(){dsn:=fmt.Sprintf("%s:%s@tcp(%s)/%s", "root", "123456", "127.0.0.1", "chanxuehong") dsn+="?collation=utf8mb4_bin&clientFoundRows=false&interpolateParams=false&loc=Asia%2FShanghai&maxAllowedPacket=0&multiStatements=false&parseTime=true&timeout=5000ms&time_zone=%27Asia%2FShanghai%27"db, err:=sql.Open("mysql", dsn) iferr!=nil{fmt.Println(err) return } iferr=db.Ping(); err!=nil{fmt.Println(err) return } db.SetMaxOpenConns(10) db.SetMaxIdleConns(5) db.SetConnMaxLifetime(30*time.Minute) DB=db } funcBenchmark1(b*testing.B){t:=time.Date(2020, 05, 14, 12, 37, 06, 0, Loc) varidintstmt, err:=DB.Prepare("select id from `test` where `dt`=?") iferr!=nil{b.Error(err) return } deferstmt.Close() b.ReportAllocs() b.ResetTimer() fori:=0; i<b.N; i++{stmt.QueryRow(t).Scan(&id) } }

current version result

➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 49856 1606501 ns/op 568 B/op 19 allocs/op PASS ok test1 95.362s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 55261 1380594 ns/op 568 B/op 19 allocs/op PASS ok test1 89.836s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 54924 1376231 ns/op 568 B/op 19 allocs/op PASS ok test1 89.205s ➜ test1

my version result

➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 53731 1464168 ns/op 568 B/op 19 allocs/op PASS ok test1 92.892s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 52329 1334475 ns/op 568 B/op 19 allocs/op PASS ok test1 83.872s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 48914 1445726 ns/op 568 B/op 19 allocs/op PASS ok test1 86.606s ➜ test1

---

interpolateParams=true

package main import ( "database/sql""fmt""testing""time" _ "github.com/go-sql-driver/mysql" ) varLoc=time.FixedZone("Asia/Shanghai", 8*60*60) varDB*sql.DBfuncinit(){dsn:=fmt.Sprintf("%s:%s@tcp(%s)/%s", "root", "123456", "127.0.0.1", "chanxuehong") dsn+="?collation=utf8mb4_bin&clientFoundRows=false&interpolateParams=true&loc=Asia%2FShanghai&maxAllowedPacket=0&multiStatements=false&parseTime=true&timeout=5000ms&time_zone=%27Asia%2FShanghai%27"db, err:=sql.Open("mysql", dsn) iferr!=nil{fmt.Println(err) return } iferr=db.Ping(); err!=nil{fmt.Println(err) return } db.SetMaxOpenConns(10) db.SetMaxIdleConns(5) db.SetConnMaxLifetime(30*time.Minute) DB=db } funcBenchmark1(b*testing.B){t:=time.Date(2020, 05, 14, 12, 37, 06, 0, Loc) varidintb.ReportAllocs() b.ResetTimer() fori:=0; i<b.N; i++{DB.QueryRow("select id from `test` where `dt`=?", t).Scan(&id) } }

current version result

➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 53230 1401655 ns/op 618 B/op 19 allocs/op PASS ok test1 88.741s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 49360 1672014 ns/op 618 B/op 19 allocs/op PASS ok test1 97.629s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 49689 1416421 ns/op 618 B/op 19 allocs/op PASS ok test1 85.374s ➜ test1

my version result

➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 52440 1345134 ns/op 618 B/op 19 allocs/op PASS ok test1 84.998s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 52258 1361504 ns/op 618 B/op 19 allocs/op PASS ok test1 85.441s ➜ test1 go test -bench=. -benchtime=1m goos: darwin goarch: amd64 pkg: test1 Benchmark1-8 50906 1389503 ns/op 618 B/op 19 allocs/op PASS ok test1 85.404s ➜ test1

[chanxuehong]

Please include macro benchmark (benchmark through executing SQL) too for both of setting interpolateParams false and true.
Without it, we can't know this is real improvement.
Especially, this PR seems to increase allocation of interpolateParams.

This pr optimizes the following places：

1. https://github.com/go-sql-driver/mysql/pull/1118/files#diff-8f8a79f3f89c69a28dd878de98384cf9L249
   old version time.Add called every times, new version save 50% costs

https://github.com/go-sql-driver/mysql/pull/1118/files#diff-8f8a79f3f89c69a28dd878de98384cf9L250
old version call Year, Month, Day, Hour, Minute, Second method,
new version call Date, Clock instead, see
https://github.com/go-sql-driver/mysql/pull/1118/files#diff-df428c63426402bd3667b15209ed395fR365
https://github.com/go-sql-driver/mysql/pull/1118/files#diff-df428c63426402bd3667b15209ed395fR379

3. https://github.com/go-sql-driver/mysql/pull/1118/files#diff-2357b8494bbd2f27c09e61fc8ef5f092L1119
   time.Time.AppendFormat is slow, see
   https://github.com/go-sql-driver/mysql/pull/1118/files#diff-df428c63426402bd3667b15209ed395fR389
   https://github.com/go-sql-driver/mysql/pull/1118/files#diff-df428c63426402bd3667b15209ed395fR398

4. old version will panic if year greater than 9999 when interpolateParams=true

[dolmen]

To ease review there should have been at least 2 separate commits : one for refactoring (moving the formatting code to a function), and one for the formatting implementation change.

[dolmen]

As the aim of this PR is speed, it should keep one feature of the original implementation : appending the formatted time to an existing []byte to avoid temporary []byte allocation and copy.

So I think that formatDateTime(t time.Time) (buf [32]byte, n int, err error) should be replaced by a appendDateTime(buf []byte, t time.Time) (err error). If formatDateTime is still needed it could be rebuilt as a wrapper of appendDateTime that just allocates the buffer.

[chanxuehong]

As the aim of this PR is speed, it should keep one feature of the original implementation : appending the formatted time to an existing []byte to avoid temporary []byte allocation and copy.

So I think that formatDateTime(t time.Time) (buf [32]byte, n int, err error) should be replaced by a appendDateTime(buf []byte, t time.Time) (err error). If formatDateTime is still needed it could be rebuilt as a wrapper of appendDateTime that just allocates the buffer.

Thank you for your comment, I have changed it, please review it again