From 833702e8c45b8a81b319600611acefdda593f217 Mon Sep 17 00:00:00 2001
From: Charlie Vieth <charlie.vieth@gmail.com>
Date: Tue, 5 Nov 2024 12:01:31 -0500
Subject: [PATCH] reduce allocations when binding string/time args
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit reduces the number of allocations required to bind args by
eliminating string to byte slice conversions for string and time.Time
types and by only checking for bind parameters if any of the
driver.NamedValue args are named. It also changes bind to only reset the
sqlite3 statement when necessary - previously the statement was always
reset even on first use (this yields a 3-4% performance boost).

goos: darwin
goarch: arm64
pkg: github.com/mattn/go-sqlite3
cpu: Apple M4 Pro
                         │   y1.txt    │               y2.txt               │
                         │   sec/op    │   sec/op     vs base               │
Suite/BenchmarkQuery-14    2.080µ ± 3%   1.995µ ± 0%  -4.13% (p=0.000 n=10)
Suite/BenchmarkParams-14   2.282µ ± 1%   2.181µ ± 2%  -4.43% (p=0.000 n=10)
Suite/BenchmarkStmt-14     1.537µ ± 1%   1.489µ ± 1%  -3.16% (p=0.000 n=10)
geomean                    1.939µ        1.864µ       -3.91%

                         │   y1.txt    │                y2.txt                 │
                         │    B/op     │    B/op      vs base                  │
Suite/BenchmarkQuery-14     688.0 ± 0%    688.0 ± 0%        ~ (p=1.000 n=10) ¹
Suite/BenchmarkParams-14   1104.0 ± 0%   1000.0 ± 0%   -9.42% (p=0.000 n=10)
Suite/BenchmarkStmt-14      920.0 ± 0%    816.0 ± 0%  -11.30% (p=0.000 n=10)
geomean                     887.4         824.9        -7.04%
¹ all samples are equal

                         │   y1.txt   │               y2.txt                │
                         │ allocs/op  │ allocs/op   vs base                 │
Suite/BenchmarkQuery-14    23.00 ± 0%   23.00 ± 0%       ~ (p=1.000 n=10) ¹
Suite/BenchmarkParams-14   27.00 ± 0%   25.00 ± 0%  -7.41% (p=0.000 n=10)
Suite/BenchmarkStmt-14     25.00 ± 0%   23.00 ± 0%  -8.00% (p=0.000 n=10)
geomean                    24.95        23.65       -5.20%
¹ all samples are equal
---
 sqlite3.go      | 107 +++++++++++++++++++++++++++++++++++++++---------
 unsafe_go120.go |  17 ++++++++
 unsafe_go121.go |  23 +++++++++++
 3 files changed, 128 insertions(+), 19 deletions(-)
 create mode 100644 unsafe_go120.go
 create mode 100644 unsafe_go121.go

diff --git a/sqlite3.go b/sqlite3.go
index 3025a500..3b717720 100644
--- a/sqlite3.go
+++ b/sqlite3.go
@@ -382,6 +382,7 @@ type SQLiteStmt struct {
 	t      string
 	closed bool
 	cls    bool // True if the statement was created by SQLiteConn.Query
+	reset  bool // True if the statement needs to reset before reuse
 }
 
 // SQLiteResult implements sql.Result.
@@ -1921,26 +1922,97 @@ func (s *SQLiteStmt) NumInput() int {
 
 var placeHolder = []byte{0}
 
+func hasNamedArgs(args []driver.NamedValue) bool {
+	for _, v := range args {
+		if v.Name != "" {
+			return true
+		}
+	}
+	return false
+}
+
 func (s *SQLiteStmt) bind(args []driver.NamedValue) error {
-	rv := C.sqlite3_reset(s.s)
-	if rv != C.SQLITE_ROW && rv != C.SQLITE_OK && rv != C.SQLITE_DONE {
-		return s.c.lastError()
+	if s.reset {
+		// The statement was previously used so we need to reset it.
+		rv := C.sqlite3_reset(s.s)
+		if rv != C.SQLITE_ROW && rv != C.SQLITE_OK && rv != C.SQLITE_DONE {
+			return s.c.lastError()
+		}
+	} else {
+		// First call to bind, future calls will need to reset the statement.
+		s.reset = true
+	}
+
+	if hasNamedArgs(args) {
+		return s.bindIndices(args)
+	}
+
+	var rv C.int
+	for _, arg := range args {
+		n := C.int(arg.Ordinal)
+		switch v := arg.Value.(type) {
+		case nil:
+			rv = C.sqlite3_bind_null(s.s, n)
+		case string:
+			p := stringData(v)
+			rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(p)), C.int(len(v)))
+		case int64:
+			rv = C.sqlite3_bind_int64(s.s, n, C.sqlite3_int64(v))
+		case bool:
+			val := 0
+			if v {
+				val = 1
+			}
+			rv = C.sqlite3_bind_int(s.s, n, C.int(val))
+		case float64:
+			rv = C.sqlite3_bind_double(s.s, n, C.double(v))
+		case []byte:
+			if v == nil {
+				rv = C.sqlite3_bind_null(s.s, n)
+			} else {
+				ln := len(v)
+				if ln == 0 {
+					v = placeHolder
+				}
+				rv = C._sqlite3_bind_blob(s.s, n, unsafe.Pointer(&v[0]), C.int(ln))
+			}
+		case time.Time:
+			ts := v.Format(SQLiteTimestampFormats[0])
+			p := stringData(ts)
+			rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(p)), C.int(len(ts)))
+		}
+		if rv != C.SQLITE_OK {
+			return s.c.lastError()
+		}
 	}
+	return nil
+}
+
+func (s *SQLiteStmt) bindIndices(args []driver.NamedValue) error {
+	// Find the longest named parameter name.
+	n := 0
+	for _, v := range args {
+		if m := len(v.Name); m > n {
+			n = m
+		}
+	}
+	buf := make([]byte, 0, n+2) // +2 for placeholder and null terminator
 
 	bindIndices := make([][3]int, len(args))
-	prefixes := []string{":", "@", "$"}
 	for i, v := range args {
 		bindIndices[i][0] = args[i].Ordinal
 		if v.Name != "" {
-			for j := range prefixes {
-				cname := C.CString(prefixes[j] + v.Name)
-				bindIndices[i][j] = int(C.sqlite3_bind_parameter_index(s.s, cname))
-				C.free(unsafe.Pointer(cname))
+			for j, c := range []byte{':', '@', '$'} {
+				buf = append(buf[:0], c)
+				buf = append(buf, v.Name...)
+				buf = append(buf, 0)
+				bindIndices[i][j] = int(C.sqlite3_bind_parameter_index(s.s, (*C.char)(unsafe.Pointer(&buf[0]))))
 			}
 			args[i].Ordinal = bindIndices[i][0]
 		}
 	}
 
+	var rv C.int
 	for i, arg := range args {
 		for j := range bindIndices[i] {
 			if bindIndices[i][j] == 0 {
@@ -1951,20 +2023,16 @@ func (s *SQLiteStmt) bind(args []driver.NamedValue) error {
 			case nil:
 				rv = C.sqlite3_bind_null(s.s, n)
 			case string:
-				if len(v) == 0 {
-					rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&placeHolder[0])), C.int(0))
-				} else {
-					b := []byte(v)
-					rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&b[0])), C.int(len(b)))
-				}
+				p := stringData(v)
+				rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(p)), C.int(len(v)))
 			case int64:
 				rv = C.sqlite3_bind_int64(s.s, n, C.sqlite3_int64(v))
 			case bool:
+				val := 0
 				if v {
-					rv = C.sqlite3_bind_int(s.s, n, 1)
-				} else {
-					rv = C.sqlite3_bind_int(s.s, n, 0)
+					val = 1
 				}
+				rv = C.sqlite3_bind_int(s.s, n, C.int(val))
 			case float64:
 				rv = C.sqlite3_bind_double(s.s, n, C.double(v))
 			case []byte:
@@ -1978,8 +2046,9 @@ func (s *SQLiteStmt) bind(args []driver.NamedValue) error {
 					rv = C._sqlite3_bind_blob(s.s, n, unsafe.Pointer(&v[0]), C.int(ln))
 				}
 			case time.Time:
-				b := []byte(v.Format(SQLiteTimestampFormats[0]))
-				rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&b[0])), C.int(len(b)))
+				ts := v.Format(SQLiteTimestampFormats[0])
+				p := stringData(ts)
+				rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(p)), C.int(len(ts)))
 			}
 			if rv != C.SQLITE_OK {
 				return s.c.lastError()
diff --git a/unsafe_go120.go b/unsafe_go120.go
new file mode 100644
index 00000000..95d673ed
--- /dev/null
+++ b/unsafe_go120.go
@@ -0,0 +1,17 @@
+//go:build !go1.21
+// +build !go1.21
+
+package sqlite3
+
+import "unsafe"
+
+// stringData is a safe version of unsafe.StringData that handles empty strings.
+func stringData(s string) *byte {
+	if len(s) != 0 {
+		b := *(*[]byte)(unsafe.Pointer(&s))
+		return &b[0]
+	}
+	// The return value of unsafe.StringData
+	// is unspecified if the string is empty.
+	return &placeHolder[0]
+}
diff --git a/unsafe_go121.go b/unsafe_go121.go
new file mode 100644
index 00000000..b9c00a12
--- /dev/null
+++ b/unsafe_go121.go
@@ -0,0 +1,23 @@
+//go:build go1.21
+// +build go1.21
+
+// The unsafe.StringData function was made available in Go 1.20 but it
+// was not until Go 1.21 that Go was changed to interpret the Go version
+// in go.mod (1.19 as of writing this) as the minimum version required
+// instead of the exact version.
+//
+// See: https://github.com/golang/go/issues/59033
+
+package sqlite3
+
+import "unsafe"
+
+// stringData is a safe version of unsafe.StringData that handles empty strings.
+func stringData(s string) *byte {
+	if len(s) != 0 {
+		return unsafe.StringData(s)
+	}
+	// The return value of unsafe.StringData
+	// is unspecified if the string is empty.
+	return &placeHolder[0]
+}