Merge upstream

.coderabbit.yaml

@@ -0,0 +1,15 @@
+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+language: en-US
+reviews:
+  # Skip the vendored SQLite amalgamation. These files are copied verbatim from
+  # upstream SQLite (see the License section in README.md) and are not code that
+  # this project authors or reviews.
+  path_filters:
+    - "!sqlite3-binding.c"
+    - "!sqlite3-binding.h"
+    - "!sqlite3ext.h"
+  auto_review:
+    enabled: true
+    drafts: false
+chat:
+  auto_reply: true

README.md

@@ -3,4 +3,3 @@ go-sqlite3
 
 [![Circle CI](https://circleci.com/gh/rqlite/go-sqlite3/tree/master.svg?style=svg)](https://circleci.com/gh/rqlite/go-sqlite3/tree/master)
 
-See upstream for README.

callback.go

@@ -29,12 +29,13 @@ import (
 	"math"
 	"reflect"
 	"sync"
+	"sync/atomic"
 	"unsafe"
 )
 
 //export callbackTrampoline
-func callbackTrampoline(ctx *C.sqlite3_context, argc int, argv **C.sqlite3_value) {
-	args := (*[(math.MaxInt32 - 1) / unsafe.Sizeof((*C.sqlite3_value)(nil))]*C.sqlite3_value)(unsafe.Pointer(argv))[:argc:argc]
+func callbackTrampoline(ctx *C.sqlite3_context, argc C.int, argv **C.sqlite3_value) {
+	args := (*[(math.MaxInt32 - 1) / unsafe.Sizeof((*C.sqlite3_value)(nil))]*C.sqlite3_value)(unsafe.Pointer(argv))[:int(argc):int(argc)]
 	fi := lookupHandle(C.sqlite3_user_data(ctx)).(*functionInfo)
 	fi.Call(ctx, args)
 }
@@ -59,9 +60,9 @@ func compareTrampoline(handlePtr unsafe.Pointer, la C.int, a *C.char, lb C.int,
 }
 
 //export commitHookTrampoline
-func commitHookTrampoline(handle unsafe.Pointer) int {
+func commitHookTrampoline(handle unsafe.Pointer) C.int {
 	callback := lookupHandle(handle).(func() int)
-	return callback()
+	return C.int(callback())
 }
 
 //export rollbackHookTrampoline
@@ -71,23 +72,23 @@ func rollbackHookTrampoline(handle unsafe.Pointer) {
 }
 
 //export updateHookTrampoline
-func updateHookTrampoline(handle unsafe.Pointer, op int, db *C.char, table *C.char, rowid int64) {
+func updateHookTrampoline(handle unsafe.Pointer, op C.int, db *C.char, table *C.char, rowid int64) {
 	callback := lookupHandle(handle).(func(int, string, string, int64))
-	callback(op, C.GoString(db), C.GoString(table), rowid)
+	callback(int(op), C.GoString(db), C.GoString(table), rowid)
 }
 
 //export authorizerTrampoline
-func authorizerTrampoline(handle unsafe.Pointer, op int, arg1 *C.char, arg2 *C.char, arg3 *C.char) int {
+func authorizerTrampoline(handle unsafe.Pointer, op C.int, arg1 *C.char, arg2 *C.char, arg3 *C.char) C.int {
 	callback := lookupHandle(handle).(func(int, string, string, string) int)
-	return callback(op, C.GoString(arg1), C.GoString(arg2), C.GoString(arg3))
+	return C.int(callback(int(op), C.GoString(arg1), C.GoString(arg2), C.GoString(arg3)))
 }
 
 //export preUpdateHookTrampoline
-func preUpdateHookTrampoline(handle unsafe.Pointer, dbHandle uintptr, op int, db *C.char, table *C.char, oldrowid int64, newrowid int64) {
+func preUpdateHookTrampoline(handle unsafe.Pointer, dbHandle uintptr, op C.int, db *C.char, table *C.char, oldrowid int64, newrowid int64) {
 	hval := lookupHandleVal(handle)
 	data := SQLitePreUpdateData{
 		Conn:         hval.db,
-		Op:           op,
+		Op:           int(op),
 		DatabaseName: C.GoString(db),
 		TableName:    C.GoString(table),
 		OldRowID:     oldrowid,
@@ -104,24 +105,26 @@ type handleVal struct {
 }
 
 var handleLock sync.Mutex
-var handleVals = make(map[unsafe.Pointer]handleVal)
+var handleVals atomic.Value // stores map[unsafe.Pointer]handleVal
 
 func newHandle(db *SQLiteConn, v any) unsafe.Pointer {
-	handleLock.Lock()
-	defer handleLock.Unlock()
 	val := handleVal{db: db, val: v}
 	var p unsafe.Pointer = C.malloc(C.size_t(1))
 	if p == nil {
 		panic("can't allocate 'cgo-pointer hack index pointer': ptr == nil")
 	}
-	handleVals[p] = val
+
+	handleLock.Lock()
+	defer handleLock.Unlock()
+
+	next := cloneHandleVals(len(loadHandleVals()) + 1)
+	next[p] = val
+	handleVals.Store(next)
 	return p
 }
 
 func lookupHandleVal(handle unsafe.Pointer) handleVal {
-	handleLock.Lock()
-	defer handleLock.Unlock()
-	return handleVals[handle]
+	return loadHandleVals()[handle]
 }
 
 func lookupHandle(handle unsafe.Pointer) any {
@@ -131,12 +134,34 @@ func lookupHandle(handle unsafe.Pointer) any {
 func deleteHandles(db *SQLiteConn) {
 	handleLock.Lock()
 	defer handleLock.Unlock()
-	for handle, val := range handleVals {
+
+	current := loadHandleVals()
+	if len(current) == 0 {
+		return
+	}
+
+	next := make(map[unsafe.Pointer]handleVal, len(current))
+	for handle, val := range current {
 		if val.db == db {
-			delete(handleVals, handle)
 			C.free(handle)
+			continue
 		}
+		next[handle] = val
+	}
+	handleVals.Store(next)
+}
+
+func loadHandleVals() map[unsafe.Pointer]handleVal {
+	m, _ := handleVals.Load().(map[unsafe.Pointer]handleVal)
+	return m
+}
+
+func cloneHandleVals(size int) map[unsafe.Pointer]handleVal {
+	next := make(map[unsafe.Pointer]handleVal, size)
+	for handle, val := range loadHandleVals() {
+		next[handle] = val
 	}
+	return next
 }
 
 // This is only here so that tests can refer to it.
@@ -235,6 +260,16 @@ func callbackArgGeneric(v *C.sqlite3_value) (reflect.Value, error) {
 	}
 }
 
+// callbackArgConvert returns conv as-is when the parameter type is the
+// canonical type conv produces, and wraps it with a cast for named types
+// (e.g. time.Duration), which reflect.Call would otherwise panic on.
+func callbackArgConvert(conv callbackArgConverter, typ, canonical reflect.Type) callbackArgConverter {
+	if typ == canonical {
+		return conv
+	}
+	return callbackArgCast{conv, typ}.Run
+}
+
 func callbackArg(typ reflect.Type) (callbackArgConverter, error) {
 	switch typ.Kind() {
 	case reflect.Interface:
@@ -246,18 +281,18 @@ func callbackArg(typ reflect.Type) (callbackArgConverter, error) {
 		if typ.Elem().Kind() != reflect.Uint8 {
 			return nil, errors.New("the only supported slice type is []byte")
 		}
-		return callbackArgBytes, nil
+		return callbackArgConvert(callbackArgBytes, typ, reflect.TypeOf([]byte(nil))), nil
 	case reflect.String:
-		return callbackArgString, nil
+		return callbackArgConvert(callbackArgString, typ, reflect.TypeOf("")), nil
 	case reflect.Bool:
-		return callbackArgBool, nil
+		return callbackArgConvert(callbackArgBool, typ, reflect.TypeOf(false)), nil
 	case reflect.Int64:
-		return callbackArgInt64, nil
+		return callbackArgConvert(callbackArgInt64, typ, reflect.TypeOf(int64(0))), nil
 	case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Int, reflect.Uint:
 		c := callbackArgCast{callbackArgInt64, typ}
 		return c.Run, nil
 	case reflect.Float64:
-		return callbackArgFloat64, nil
+		return callbackArgConvert(callbackArgFloat64, typ, reflect.TypeOf(float64(0))), nil
 	case reflect.Float32:
 		c := callbackArgCast{callbackArgFloat64, typ}
 		return c.Run, nil
@@ -301,8 +336,7 @@ func callbackRetInteger(ctx *C.sqlite3_context, v reflect.Value) error {
 	case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Int, reflect.Uint:
 		v = v.Convert(reflect.TypeOf(int64(0)))
 	case reflect.Bool:
-		b := v.Interface().(bool)
-		if b {
+		if v.Bool() {
 			v = reflect.ValueOf(int64(1))
 		} else {
 			v = reflect.ValueOf(int64(0))
@@ -311,7 +345,7 @@ func callbackRetInteger(ctx *C.sqlite3_context, v reflect.Value) error {
 		return fmt.Errorf("cannot convert %s to INTEGER", v.Type())
 	}
 
-	C.sqlite3_result_int64(ctx, C.sqlite3_int64(v.Interface().(int64)))
+	C.sqlite3_result_int64(ctx, C.sqlite3_int64(v.Int()))
 	return nil
 }
 
@@ -324,19 +358,18 @@ func callbackRetFloat(ctx *C.sqlite3_context, v reflect.Value) error {
 		return fmt.Errorf("cannot convert %s to FLOAT", v.Type())
 	}
 
-	C.sqlite3_result_double(ctx, C.double(v.Interface().(float64)))
+	C.sqlite3_result_double(ctx, C.double(v.Float()))
 	return nil
 }
 
 func callbackRetBlob(ctx *C.sqlite3_context, v reflect.Value) error {
 	if v.Type().Kind() != reflect.Slice || v.Type().Elem().Kind() != reflect.Uint8 {
 		return fmt.Errorf("cannot convert %s to BLOB", v.Type())
 	}
-	i := v.Interface()
-	if i == nil || len(i.([]byte)) == 0 {
+	bs := v.Bytes()
+	if len(bs) == 0 {
 		C.sqlite3_result_null(ctx)
 	} else {
-		bs := i.([]byte)
 		if i64 && len(bs) > math.MaxInt32 {
 			C.sqlite3_result_error_toobig(ctx)
 			return nil
@@ -350,7 +383,7 @@ func callbackRetText(ctx *C.sqlite3_context, v reflect.Value) error {
 	if v.Type().Kind() != reflect.String {
 		return fmt.Errorf("cannot convert %s to TEXT", v.Type())
 	}
-	s := v.Interface().(string)
+	s := v.String()
 	if i64 && len(s) > math.MaxInt32 {
 		C.sqlite3_result_error_toobig(ctx)
 		return nil

callback_bench_test.go

@@ -0,0 +1,85 @@
+// Copyright (C) 2019 Yasuhiro Matsumoto <mattn.jp@gmail.com>.
+//
+// Use of this source code is governed by an MIT-style
+// license that can be found in the LICENSE file.
+
+//go:build cgo
+// +build cgo
+
+package sqlite3
+
+import (
+	"sync"
+	"sync/atomic"
+	"testing"
+	"unsafe"
+)
+
+func BenchmarkHandleLookupParallel(b *testing.B) {
+	d := SQLiteDriver{}
+	conn, err := d.Open(":memory:")
+	if err != nil {
+		b.Fatal(err)
+	}
+	defer conn.Close()
+	c := conn.(*SQLiteConn)
+
+	handle := newHandle(c, func() {})
+
+	benchmarkHandleLookupParallel(b, func() any {
+		return lookupHandle(handle)
+	})
+}
+
+func BenchmarkHandleLookupBeforeAfter(b *testing.B) {
+	value := handleVal{val: func() {}}
+	handle := unsafe.Pointer(&value)
+
+	before := mutexHandleTable{vals: map[unsafe.Pointer]handleVal{handle: value}}
+	after := atomicHandleTable{}
+	after.vals.Store(map[unsafe.Pointer]handleVal{handle: value})
+
+	b.Run("before_mutex", func(b *testing.B) {
+		benchmarkHandleLookupParallel(b, func() any {
+			return before.lookup(handle).val
+		})
+	})
+	b.Run("after_atomic", func(b *testing.B) {
+		benchmarkHandleLookupParallel(b, func() any {
+			return after.lookup(handle).val
+		})
+	})
+}
+
+func benchmarkHandleLookupParallel(b *testing.B, lookup func() any) {
+	b.Helper()
+	b.ReportAllocs()
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			if lookup() == nil {
+				b.Fatal("lookup returned nil")
+			}
+		}
+	})
+}
+
+type mutexHandleTable struct {
+	mu   sync.Mutex
+	vals map[unsafe.Pointer]handleVal
+}
+
+func (t *mutexHandleTable) lookup(handle unsafe.Pointer) handleVal {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	return t.vals[handle]
+}
+
+type atomicHandleTable struct {
+	vals atomic.Value
+}
+
+func (t *atomicHandleTable) lookup(handle unsafe.Pointer) handleVal {
+	m, _ := t.vals.Load().(map[unsafe.Pointer]handleVal)
+	return m[handle]
+}