Skip to content

Commit 0861724

Browse files
committed
Fix exponential memory allocation in Exec and improve performance
This commit changes SQLiteConn.Exec to use the raw Go query string instead of repeatedly converting it to a C string (which it would do for every statement in the provided query). This yields a ~20% performance improvement for a query containing one statement and a significantly larger improvement when the query contains multiple statements as is common when importing a SQL dump (our benchmark shows a 5x improvement for handling 1k SQL statements). Additionally, this commit improves the performance of Exec by 2x or more and makes number and size of allocations constant when there are no bind parameters (the performance improvement scales with the number of SQL statements in the query). This is achieved by having the entire query processed in C code thus requiring only one CGO call. The speedup for Exec'ing single statement queries means that wrapping simple statements in a transaction is now twice as fast. This commit also improves the test coverage of Exec, which previously failed to test that Exec could process multiple statements like INSERT. It also adds some Exec specific benchmarks that highlight both the improvements here and the overhead of using a cancellable Context. This commit is a slimmed down and improved version of PR mattn#1133: mattn#1133 ``` goos: darwin goarch: arm64 pkg: github.com/mattn/go-sqlite3 cpu: Apple M1 Max │ b.txt │ n.txt │ │ sec/op │ sec/op vs base │ Suite/BenchmarkExec/Params-10 1.434µ ± 1% 1.186µ ± 0% -17.27% (p=0.000 n=10) Suite/BenchmarkExec/NoParams-10 1267.5n ± 0% 759.2n ± 1% -40.10% (p=0.000 n=10) Suite/BenchmarkExecContext/Params-10 2.886µ ± 0% 2.517µ ± 0% -12.80% (p=0.000 n=10) Suite/BenchmarkExecContext/NoParams-10 2.605µ ± 1% 1.829µ ± 1% -29.81% (p=0.000 n=10) Suite/BenchmarkExecStep-10 1852.6µ ± 1% 582.3µ ± 0% -68.57% (p=0.000 n=10) Suite/BenchmarkExecContextStep-10 3053.3µ ± 3% 582.0µ ± 0% -80.94% (p=0.000 n=10) Suite/BenchmarkExecTx-10 4.126µ ± 2% 2.200µ ± 1% -46.67% (p=0.000 n=10) geomean 16.40µ 8.455µ -48.44% │ b.txt │ n.txt │ │ B/op │ B/op vs base │ Suite/BenchmarkExec/Params-10 248.0 ± 0% 240.0 ± 0% -3.23% (p=0.000 n=10) Suite/BenchmarkExec/NoParams-10 128.00 ± 0% 64.00 ± 0% -50.00% (p=0.000 n=10) Suite/BenchmarkExecContext/Params-10 408.0 ± 0% 400.0 ± 0% -1.96% (p=0.000 n=10) Suite/BenchmarkExecContext/NoParams-10 288.0 ± 0% 208.0 ± 0% -27.78% (p=0.000 n=10) Suite/BenchmarkExecStep-10 5406674.50 ± 0% 64.00 ± 0% -100.00% (p=0.000 n=10) Suite/BenchmarkExecContextStep-10 5566758.5 ± 0% 208.0 ± 0% -100.00% (p=0.000 n=10) Suite/BenchmarkExecTx-10 712.0 ± 0% 520.0 ± 0% -26.97% (p=0.000 n=10) geomean 4.899Ki 189.7 -96.22% │ b.txt │ n.txt │ │ allocs/op │ allocs/op vs base │ Suite/BenchmarkExec/Params-10 10.000 ± 0% 9.000 ± 0% -10.00% (p=0.000 n=10) Suite/BenchmarkExec/NoParams-10 7.000 ± 0% 4.000 ± 0% -42.86% (p=0.000 n=10) Suite/BenchmarkExecContext/Params-10 12.00 ± 0% 11.00 ± 0% -8.33% (p=0.000 n=10) Suite/BenchmarkExecContext/NoParams-10 9.000 ± 0% 6.000 ± 0% -33.33% (p=0.000 n=10) Suite/BenchmarkExecStep-10 7000.000 ± 0% 4.000 ± 0% -99.94% (p=0.000 n=10) Suite/BenchmarkExecContextStep-10 9001.000 ± 0% 6.000 ± 0% -99.93% (p=0.000 n=10) Suite/BenchmarkExecTx-10 27.00 ± 0% 18.00 ± 0% -33.33% (p=0.000 n=10) geomean 74.60 7.224 -90.32% ```
1 parent d95e7e5 commit 0861724

File tree

3 files changed

+184
-22
lines changed

3 files changed

+184
-22
lines changed

sqlite3.go

Lines changed: 144 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,53 @@ _sqlite3_prepare_v2_internal(sqlite3 *db, const char *zSql, int nBytes, sqlite3_
137137
}
138138
#endif
139139
140+
static int _sqlite3_prepare_v2(sqlite3 *db, const char *zSql, int nBytes, sqlite3_stmt **ppStmt, int *oBytes) {
141+
const char *tail = NULL;
142+
int rv = _sqlite3_prepare_v2_internal(db, zSql, nBytes, ppStmt, &tail);
143+
if (rv != SQLITE_OK) {
144+
return rv;
145+
}
146+
if (tail == NULL) {
147+
return rv; // NB: this should not happen
148+
}
149+
// Set oBytes to the number of bytes consumed instead of using the **pzTail
150+
// out param since that requires storing a Go pointer in a C pointer, which
151+
// is not allowed by CGO and will cause runtime.cgoCheckPointer to fail.
152+
*oBytes = tail - zSql;
153+
return rv;
154+
}
155+
156+
// _sqlite3_exec_no_args executes all of the statements in zSql. None of the
157+
// statements are allowed to have positional arguments.
158+
int _sqlite3_exec_no_args(sqlite3 *db, const char *zSql, int nBytes, int64_t *rowid, int64_t *changes) {
159+
while (*zSql && nBytes > 0) {
160+
sqlite3_stmt *stmt;
161+
const char *tail;
162+
int rv = sqlite3_prepare_v2(db, zSql, nBytes, &stmt, &tail);
163+
if (rv != SQLITE_OK) {
164+
return rv;
165+
}
166+
167+
// Process statement
168+
do {
169+
rv = _sqlite3_step_internal(stmt);
170+
} while (rv == SQLITE_ROW);
171+
172+
// Only record the number of changes made by the last statement.
173+
*changes = sqlite3_changes64(db);
174+
*rowid = sqlite3_last_insert_rowid(db);
175+
176+
sqlite3_finalize(stmt);
177+
if (rv != SQLITE_OK && rv != SQLITE_DONE) {
178+
return rv;
179+
}
180+
181+
nBytes -= tail - zSql;
182+
zSql = tail;
183+
}
184+
return SQLITE_OK;
185+
}
186+
140187
void _sqlite3_result_text(sqlite3_context* ctx, const char* s) {
141188
sqlite3_result_text(ctx, s, -1, &free);
142189
}
@@ -921,54 +968,122 @@ func (c *SQLiteConn) Exec(query string, args []driver.Value) (driver.Result, err
921968
}
922969

923970
func (c *SQLiteConn) exec(ctx context.Context, query string, args []driver.NamedValue) (driver.Result, error) {
924-
start := 0
971+
// Trim the query. This is mostly important for getting rid
972+
// of any trailing space.
973+
query = strings.TrimSpace(query)
974+
if len(args) > 0 {
975+
return c.execArgs(ctx, query, args)
976+
}
977+
return c.execNoArgs(ctx, query)
978+
}
979+
980+
func (c *SQLiteConn) execArgs(ctx context.Context, query string, args []driver.NamedValue) (driver.Result, error) {
981+
var (
982+
stmtArgs []driver.NamedValue
983+
start int
984+
s SQLiteStmt // escapes to the heap so reuse it
985+
sz C.int // number of query bytes consumed: escapes to the heap
986+
)
925987
for {
926-
s, err := c.prepare(ctx, query)
927-
if err != nil {
928-
return nil, err
988+
s = SQLiteStmt{c: c} // reset
989+
sz = 0
990+
rv := C._sqlite3_prepare_v2(c.db, (*C.char)(unsafe.Pointer(stringData(query))),
991+
C.int(len(query)), &s.s, &sz)
992+
if rv != C.SQLITE_OK {
993+
return nil, c.lastError()
929994
}
995+
query = strings.TrimSpace(query[sz:])
996+
930997
var res driver.Result
931-
if s.(*SQLiteStmt).s != nil {
932-
stmtArgs := make([]driver.NamedValue, 0, len(args))
998+
if s.s != nil {
933999
na := s.NumInput()
9341000
if len(args)-start < na {
935-
s.Close()
1001+
s.finalize()
9361002
return nil, fmt.Errorf("not enough args to execute query: want %d got %d", na, len(args))
9371003
}
9381004
// consume the number of arguments used in the current
9391005
// statement and append all named arguments not
9401006
// contained therein
941-
if len(args[start:start+na]) > 0 {
942-
stmtArgs = append(stmtArgs, args[start:start+na]...)
943-
for i := range args {
944-
if (i < start || i >= na) && args[i].Name != "" {
945-
stmtArgs = append(stmtArgs, args[i])
946-
}
947-
}
948-
for i := range stmtArgs {
949-
stmtArgs[i].Ordinal = i + 1
1007+
if stmtArgs == nil {
1008+
stmtArgs = make([]driver.NamedValue, 0, na)
1009+
}
1010+
stmtArgs = append(stmtArgs[:0], args[start:start+na]...)
1011+
for i := range args {
1012+
if (i < start || i >= na) && args[i].Name != "" {
1013+
stmtArgs = append(stmtArgs, args[i])
9501014
}
9511015
}
952-
res, err = s.(*SQLiteStmt).exec(ctx, stmtArgs)
1016+
for i := range stmtArgs {
1017+
stmtArgs[i].Ordinal = i + 1
1018+
}
1019+
var err error
1020+
res, err = s.exec(ctx, stmtArgs)
9531021
if err != nil && err != driver.ErrSkip {
954-
s.Close()
1022+
s.finalize()
9551023
return nil, err
9561024
}
9571025
start += na
9581026
}
959-
tail := s.(*SQLiteStmt).t
960-
s.Close()
961-
if tail == "" {
1027+
s.finalize()
1028+
if len(query) == 0 {
9621029
if res == nil {
9631030
// https://github.com/mattn/go-sqlite3/issues/963
9641031
res = &SQLiteResult{0, 0}
9651032
}
9661033
return res, nil
9671034
}
968-
query = tail
9691035
}
9701036
}
9711037

1038+
// execNoArgsSync processes every SQL statement in query. All processing occurs
1039+
// in C code, which reduces the overhead of CGO calls.
1040+
func (c *SQLiteConn) execNoArgsSync(query string) (_ driver.Result, err error) {
1041+
var rowid, changes C.int64_t
1042+
rv := C._sqlite3_exec_no_args(c.db, (*C.char)(unsafe.Pointer(stringData(query))),
1043+
C.int(len(query)), &rowid, &changes)
1044+
if rv != C.SQLITE_OK {
1045+
err = c.lastError()
1046+
}
1047+
return &SQLiteResult{id: int64(rowid), changes: int64(changes)}, err
1048+
}
1049+
1050+
func (c *SQLiteConn) execNoArgs(ctx context.Context, query string) (driver.Result, error) {
1051+
done := ctx.Done()
1052+
if done == nil {
1053+
return c.execNoArgsSync(query)
1054+
}
1055+
1056+
// Fast check if the Context is cancelled
1057+
if err := ctx.Err(); err != nil {
1058+
return nil, err
1059+
}
1060+
1061+
ch := make(chan struct{})
1062+
defer close(ch)
1063+
go func() {
1064+
select {
1065+
case <-done:
1066+
C.sqlite3_interrupt(c.db)
1067+
// Wait until signaled. We need to ensure that this goroutine
1068+
// will not call interrupt after this method returns, which is
1069+
// why we can't check if only done is closed when waiting below.
1070+
<-ch
1071+
case <-ch:
1072+
}
1073+
}()
1074+
1075+
res, err := c.execNoArgsSync(query)
1076+
1077+
// Stop the goroutine and make sure we're at a point where
1078+
// sqlite3_interrupt cannot be called again.
1079+
ch <- struct{}{}
1080+
1081+
if isInterruptErr(err) {
1082+
err = ctx.Err()
1083+
}
1084+
return res, err
1085+
}
1086+
9721087
// Query implements Queryer.
9731088
func (c *SQLiteConn) Query(query string, args []driver.Value) (driver.Rows, error) {
9741089
list := make([]driver.NamedValue, len(args))
@@ -1977,6 +2092,13 @@ func (s *SQLiteStmt) Close() error {
19772092
return nil
19782093
}
19792094

2095+
func (s *SQLiteStmt) finalize() {
2096+
if s.s != nil {
2097+
C.sqlite3_finalize(s.s)
2098+
s.s = nil
2099+
}
2100+
}
2101+
19802102
// NumInput return a number of parameters.
19812103
func (s *SQLiteStmt) NumInput() int {
19822104
return int(C.sqlite3_bind_parameter_count(s.s))

unsafe_go120.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//go:build !go1.21
2+
// +build !go1.21
3+
4+
package sqlite3
5+
6+
import "unsafe"
7+
8+
// stringData is a safe version of unsafe.StringData that handles empty strings.
9+
func stringData(s string) *byte {
10+
if len(s) != 0 {
11+
b := *(*[]byte)(unsafe.Pointer(&s))
12+
return &b[0]
13+
}
14+
// The return value of unsafe.StringData
15+
// is unspecified if the string is empty.
16+
return &placeHolder[0]
17+
}

unsafe_go121.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//go:build go1.21
2+
// +build go1.21
3+
4+
// The unsafe.StringData function was made available in Go 1.20 but it
5+
// was not until Go 1.21 that Go was changed to interpret the Go version
6+
// in go.mod (1.19 as of writing this) as the minimum version required
7+
// instead of the exact version.
8+
//
9+
// See: https://github.com/golang/go/issues/59033
10+
11+
package sqlite3
12+
13+
import "unsafe"
14+
15+
// stringData is a safe version of unsafe.StringData that handles empty strings.
16+
func stringData(s string) *byte {
17+
if len(s) != 0 {
18+
return unsafe.StringData(s)
19+
}
20+
// The return value of unsafe.StringData
21+
// is unspecified if the string is empty.
22+
return &placeHolder[0]
23+
}

0 commit comments

Comments
 (0)