Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove superfluous use of runtime.SetFinalizer on SQLiteRows #1301

Merged
merged 1 commit into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions sqlite3.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ type SQLiteStmt struct {
s *C.sqlite3_stmt
t string
closed bool
cls bool
cls bool // True if the statement was created by SQLiteConn.Query
}

// SQLiteResult implements sql.Result.
Expand All @@ -393,12 +393,12 @@ type SQLiteResult struct {
// SQLiteRows implements driver.Rows.
type SQLiteRows struct {
s *SQLiteStmt
nc int
nc int32 // Number of columns
cls bool // True if we need to close the parent statement in Close
cols []string
decltype []string
cls bool
closed bool
ctx context.Context // no better alternative to pass context into Next() method
closemu sync.Mutex
}

type functionInfo struct {
Expand Down Expand Up @@ -2008,14 +2008,12 @@ func (s *SQLiteStmt) query(ctx context.Context, args []driver.NamedValue) (drive

rows := &SQLiteRows{
s: s,
nc: int(C.sqlite3_column_count(s.s)),
nc: int32(C.sqlite3_column_count(s.s)),
cls: s.cls,
cols: nil,
decltype: nil,
cls: s.cls,
closed: false,
ctx: ctx,
}
runtime.SetFinalizer(rows, (*SQLiteRows).Close)

return rows, nil
}
Expand Down Expand Up @@ -2112,34 +2110,38 @@ func (s *SQLiteStmt) Readonly() bool {

// Close the rows.
func (rc *SQLiteRows) Close() error {
rc.s.mu.Lock()
if rc.s.closed || rc.closed {
rc.s.mu.Unlock()
rc.closemu.Lock()
defer rc.closemu.Unlock()
s := rc.s
if s == nil {
return nil
}
rc.s = nil // remove reference to SQLiteStmt
s.mu.Lock()
if s.closed {
s.mu.Unlock()
return nil
}
rc.closed = true
if rc.cls {
rc.s.mu.Unlock()
return rc.s.Close()
s.mu.Unlock()
return s.Close()
}
rv := C.sqlite3_reset(rc.s.s)
rv := C.sqlite3_reset(s.s)
if rv != C.SQLITE_OK {
rc.s.mu.Unlock()
return rc.s.c.lastError()
s.mu.Unlock()
return s.c.lastError()
}
rc.s.mu.Unlock()
rc.s = nil
runtime.SetFinalizer(rc, nil)
s.mu.Unlock()
return nil
}

// Columns return column names.
func (rc *SQLiteRows) Columns() []string {
rc.s.mu.Lock()
defer rc.s.mu.Unlock()
if rc.s.s != nil && rc.nc != len(rc.cols) {
if rc.s.s != nil && int(rc.nc) != len(rc.cols) {
rc.cols = make([]string, rc.nc)
for i := 0; i < rc.nc; i++ {
for i := 0; i < int(rc.nc); i++ {
rc.cols[i] = C.GoString(C.sqlite3_column_name(rc.s.s, C.int(i)))
}
}
Expand All @@ -2149,7 +2151,7 @@ func (rc *SQLiteRows) Columns() []string {
func (rc *SQLiteRows) declTypes() []string {
if rc.s.s != nil && rc.decltype == nil {
rc.decltype = make([]string, rc.nc)
for i := 0; i < rc.nc; i++ {
for i := 0; i < int(rc.nc); i++ {
rc.decltype[i] = strings.ToLower(C.GoString(C.sqlite3_column_decltype(rc.s.s, C.int(i))))
}
}
Expand Down
18 changes: 18 additions & 0 deletions sqlite3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,7 @@ var benchmarks = []testing.InternalBenchmark{
{Name: "BenchmarkStmt", F: benchmarkStmt},
{Name: "BenchmarkRows", F: benchmarkRows},
{Name: "BenchmarkStmtRows", F: benchmarkStmtRows},
{Name: "BenchmarkQueryParallel", F: benchmarkQueryParallel},
}

func (db *TestDB) mustExec(sql string, args ...any) sql.Result {
Expand Down Expand Up @@ -2568,3 +2569,20 @@ func benchmarkStmtRows(b *testing.B) {
}
}
}

func benchmarkQueryParallel(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
db.SetMaxOpenConns(runtime.NumCPU())
defer db.Close()
var i int64
for pb.Next() {
if err := db.QueryRow("SELECT 1, 2, 3, 4").Scan(&i, &i, &i, &i); err != nil {
panic(err)
}
}
})
}
Loading