From e5d3f721f4b1a5f8f715e1f262eb2ae2be4cc85e Mon Sep 17 00:00:00 2001 From: Charlie Vieth Date: Thu, 2 Feb 2023 17:17:49 -0500 Subject: [PATCH] sqlite3: reduce C to Go string conversions in SQLiteConn.{query,exec} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes an issue where SQLiteConn.{exec,query} would use an exponential amount of memory when processing a prepared statement that consisted of multiple SQL statements ("stmt 1; stmt 2; ..."). Previously both exec/query used SQLiteConn.prepare() which converts the "tail" pointer set by sqlite3_prepare_v2() back into a Go string, which then has to be converted back to a C string for the next call to prepare() (assuming there are multiple SQL statements in the provided query). This commit fixes this by changing both exec and query to use the returned "tail" pointer for the next call to exec/query. It also changes prepare() to use pointer arithmetic to calculate the offset of the remaining "tail" portion of the query as a substring of the original query. This saves a call to C.GoString() and an allocation. Benchmarks: ``` goos: darwin goarch: arm64 pkg: github.com/mattn/go-sqlite3 │ base.10.txt │ new.10.txt │ │ sec/op │ sec/op vs base │ Suite/BenchmarkExec-10 1.351µ ± 1% 1.247µ ± 1% -7.74% (p=0.000 n=10) Suite/BenchmarkQuery-10 3.830µ ± 1% 3.558µ ± 1% -7.11% (p=0.000 n=10) Suite/BenchmarkParams-10 4.221µ ± 0% 4.228µ ± 1% ~ (p=1.000 n=10) Suite/BenchmarkStmt-10 2.906µ ± 1% 2.864µ ± 1% -1.45% (p=0.001 n=10) Suite/BenchmarkRows-10 149.1µ ± 4% 148.2µ ± 1% -0.61% (p=0.023 n=10) Suite/BenchmarkStmtRows-10 147.3µ ± 1% 145.6µ ± 0% -1.16% (p=0.000 n=10) Suite/BenchmarkExecStep-10 1898.9µ ± 3% 889.0µ ± 1% -53.18% (p=0.000 n=10) Suite/BenchmarkQueryStep-10 1848.0µ ± 1% 894.6µ ± 1% -51.59% (p=0.000 n=10) geomean 38.56µ 31.30µ -18.84% │ base.10.txt │ new.10.txt │ │ B/op │ B/op vs base │ Suite/BenchmarkExec-10 184.0 ± 0% 176.0 ± 0% -4.35% (p=0.000 n=10) Suite/BenchmarkQuery-10 864.0 ± 0% 856.0 ± 0% -0.93% (p=0.000 n=10) Suite/BenchmarkParams-10 1.289Ki ± 0% 1.281Ki ± 0% -0.61% (p=0.000 n=10) Suite/BenchmarkStmt-10 1.078Ki ± 0% 1.078Ki ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkRows-10 34.45Ki ± 0% 34.45Ki ± 0% -0.02% (p=0.000 n=10) Suite/BenchmarkStmtRows-10 34.40Ki ± 0% 34.40Ki ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecStep-10 5334.61Ki ± 0% 70.41Ki ± 0% -98.68% (p=0.000 n=10) Suite/BenchmarkQueryStep-10 5397.4Ki ± 0% 133.2Ki ± 0% -97.53% (p=0.000 n=10) geomean 17.06Ki 6.208Ki -63.62% ¹ all samples are equal │ base.10.txt │ new.10.txt │ │ allocs/op │ allocs/op vs base │ Suite/BenchmarkExec-10 13.00 ± 0% 12.00 ± 0% -7.69% (p=0.000 n=10) Suite/BenchmarkQuery-10 46.00 ± 0% 45.00 ± 0% -2.17% (p=0.000 n=10) Suite/BenchmarkParams-10 54.00 ± 0% 53.00 ± 0% -1.85% (p=0.000 n=10) Suite/BenchmarkStmt-10 49.00 ± 0% 49.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkRows-10 2.042k ± 0% 2.041k ± 0% -0.05% (p=0.000 n=10) Suite/BenchmarkStmtRows-10 2.038k ± 0% 2.038k ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecStep-10 13.000k ± 0% 8.004k ± 0% -38.43% (p=0.000 n=10) Suite/BenchmarkQueryStep-10 11.013k ± 0% 6.017k ± 0% -45.36% (p=0.000 n=10) geomean 418.6 359.8 -14.04% ¹ all samples are equal ``` --- sqlite3.go | 101 ++++++++++++++++++++++++++++++++++-------------- sqlite3_test.go | 21 ++++++++++ 2 files changed, 93 insertions(+), 29 deletions(-) diff --git a/sqlite3.go b/sqlite3.go index 5e4e2ff5..83e5e3ad 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -31,6 +31,7 @@ package sqlite3 #endif #include #include +#include #ifdef __CYGWIN__ # include @@ -79,6 +80,16 @@ _sqlite3_exec(sqlite3* db, const char* pcmd, long long* rowid, long long* change return rv; } +static const char * +_trim_leading_spaces(const char *str) { + if (str) { + while (isspace(*str)) { + str++; + } + } + return str; +} + #ifdef SQLITE_ENABLE_UNLOCK_NOTIFY extern int _sqlite3_step_blocking(sqlite3_stmt *stmt); extern int _sqlite3_step_row_blocking(sqlite3_stmt* stmt, long long* rowid, long long* changes); @@ -99,7 +110,11 @@ _sqlite3_step_row_internal(sqlite3_stmt* stmt, long long* rowid, long long* chan static int _sqlite3_prepare_v2_internal(sqlite3 *db, const char *zSql, int nBytes, sqlite3_stmt **ppStmt, const char **pzTail) { - return _sqlite3_prepare_v2_blocking(db, zSql, nBytes, ppStmt, pzTail); + int rv = _sqlite3_prepare_v2_blocking(db, zSql, nBytes, ppStmt, pzTail); + if (pzTail) { + *pzTail = _trim_leading_spaces(*pzTail); + } + return rv; } #else @@ -122,7 +137,11 @@ _sqlite3_step_row_internal(sqlite3_stmt* stmt, long long* rowid, long long* chan static int _sqlite3_prepare_v2_internal(sqlite3 *db, const char *zSql, int nBytes, sqlite3_stmt **ppStmt, const char **pzTail) { - return sqlite3_prepare_v2(db, zSql, nBytes, ppStmt, pzTail); + int rv = sqlite3_prepare_v2(db, zSql, nBytes, ppStmt, pzTail); + if (pzTail) { + *pzTail = _trim_leading_spaces(*pzTail); + } + return rv; } #endif @@ -848,24 +867,32 @@ func (c *SQLiteConn) Exec(query string, args []driver.Value) (driver.Result, err } func (c *SQLiteConn) exec(ctx context.Context, query string, args []driver.NamedValue) (driver.Result, error) { + pquery := C.CString(query) + op := pquery // original pointer + defer C.free(unsafe.Pointer(op)) + + var stmtArgs []driver.NamedValue + var tail *C.char + s := new(SQLiteStmt) // escapes to the heap so reuse it start := 0 for { - s, err := c.prepare(ctx, query) - if err != nil { - return nil, err + *s = SQLiteStmt{c: c} // reset + rv := C._sqlite3_prepare_v2_internal(c.db, pquery, C.int(-1), &s.s, &tail) + if rv != C.SQLITE_OK { + return nil, c.lastError() } + var res driver.Result - if s.(*SQLiteStmt).s != nil { - stmtArgs := make([]driver.NamedValue, 0, len(args)) + if s.s != nil { na := s.NumInput() if len(args)-start < na { - s.Close() + s.finalize() return nil, fmt.Errorf("not enough args to execute query: want %d got %d", na, len(args)) } // consume the number of arguments used in the current // statement and append all named arguments not // contained therein - stmtArgs = append(stmtArgs, args[start:start+na]...) + stmtArgs = append(stmtArgs[:0], args[start:start+na]...) for i := range args { if (i < start || i >= na) && args[i].Name != "" { stmtArgs = append(stmtArgs, args[i]) @@ -874,23 +901,23 @@ func (c *SQLiteConn) exec(ctx context.Context, query string, args []driver.Named for i := range stmtArgs { stmtArgs[i].Ordinal = i + 1 } - res, err = s.(*SQLiteStmt).exec(ctx, stmtArgs) + var err error + res, err = s.exec(ctx, stmtArgs) if err != nil && err != driver.ErrSkip { - s.Close() + s.finalize() return nil, err } start += na } - tail := s.(*SQLiteStmt).t - s.Close() - if tail == "" { + s.finalize() + if tail == nil || *tail == '\000' { if res == nil { // https://github.com/mattn/go-sqlite3/issues/963 res = &SQLiteResult{0, 0} } return res, nil } - query = tail + pquery = tail } } @@ -907,14 +934,21 @@ func (c *SQLiteConn) Query(query string, args []driver.Value) (driver.Rows, erro } func (c *SQLiteConn) query(ctx context.Context, query string, args []driver.NamedValue) (driver.Rows, error) { + pquery := C.CString(query) + op := pquery // original pointer + defer C.free(unsafe.Pointer(op)) + + var stmtArgs []driver.NamedValue + var tail *C.char + s := new(SQLiteStmt) // escapes to the heap so reuse it start := 0 for { - stmtArgs := make([]driver.NamedValue, 0, len(args)) - s, err := c.prepare(ctx, query) - if err != nil { - return nil, err + *s = SQLiteStmt{c: c, cls: true} // reset + rv := C._sqlite3_prepare_v2_internal(c.db, pquery, C.int(-1), &s.s, &tail) + if rv != C.SQLITE_OK { + return nil, c.lastError() } - s.(*SQLiteStmt).cls = true + na := s.NumInput() if len(args)-start < na { return nil, fmt.Errorf("not enough args to execute query: want %d got %d", na, len(args)-start) @@ -922,7 +956,7 @@ func (c *SQLiteConn) query(ctx context.Context, query string, args []driver.Name // consume the number of arguments used in the current // statement and append all named arguments not contained // therein - stmtArgs = append(stmtArgs, args[start:start+na]...) + stmtArgs = append(stmtArgs[:0], args[start:start+na]...) for i := range args { if (i < start || i >= na) && args[i].Name != "" { stmtArgs = append(stmtArgs, args[i]) @@ -931,19 +965,18 @@ func (c *SQLiteConn) query(ctx context.Context, query string, args []driver.Name for i := range stmtArgs { stmtArgs[i].Ordinal = i + 1 } - rows, err := s.(*SQLiteStmt).query(ctx, stmtArgs) + rows, err := s.query(ctx, stmtArgs) if err != nil && err != driver.ErrSkip { - s.Close() + s.finalize() return rows, err } start += na - tail := s.(*SQLiteStmt).t - if tail == "" { + if tail == nil || *tail == '\000' { return rows, nil } rows.Close() - s.Close() - query = tail + s.finalize() + pquery = tail } } @@ -1805,8 +1838,11 @@ func (c *SQLiteConn) prepare(ctx context.Context, query string) (driver.Stmt, er return nil, c.lastError() } var t string - if tail != nil && *tail != '\000' { - t = strings.TrimSpace(C.GoString(tail)) + if tail != nil && *tail != 0 { + n := int(uintptr(unsafe.Pointer(tail))) - int(uintptr(unsafe.Pointer(pquery))) + if 0 <= n && n < len(query) { + t = strings.TrimSpace(query[n:]) + } } ss := &SQLiteStmt{c: c, s: s, t: t} runtime.SetFinalizer(ss, (*SQLiteStmt).Close) @@ -1899,6 +1935,13 @@ func (s *SQLiteStmt) Close() error { return nil } +func (s *SQLiteStmt) finalize() { + if s.s != nil { + C.sqlite3_finalize(s.s) + s.s = nil + } +} + // NumInput return a number of parameters. func (s *SQLiteStmt) NumInput() int { return int(C.sqlite3_bind_parameter_count(s.s)) diff --git a/sqlite3_test.go b/sqlite3_test.go index 326361ec..b0d6f266 100644 --- a/sqlite3_test.go +++ b/sqlite3_test.go @@ -2111,6 +2111,8 @@ var benchmarks = []testing.InternalBenchmark{ {Name: "BenchmarkStmt", F: benchmarkStmt}, {Name: "BenchmarkRows", F: benchmarkRows}, {Name: "BenchmarkStmtRows", F: benchmarkStmtRows}, + {Name: "BenchmarkExecStep", F: benchmarkExecStep}, + {Name: "BenchmarkQueryStep", F: benchmarkQueryStep}, } func (db *TestDB) mustExec(sql string, args ...interface{}) sql.Result { @@ -2568,3 +2570,22 @@ func benchmarkStmtRows(b *testing.B) { } } } + +var largeSelectStmt = strings.Repeat("select 1;\n", 1_000) + +func benchmarkExecStep(b *testing.B) { + for n := 0; n < b.N; n++ { + if _, err := db.Exec(largeSelectStmt); err != nil { + b.Fatal(err) + } + } +} + +func benchmarkQueryStep(b *testing.B) { + var i int + for n := 0; n < b.N; n++ { + if err := db.QueryRow(largeSelectStmt).Scan(&i); err != nil { + b.Fatal(err) + } + } +}