sqlite3: simplify caching the results of sqlite3_errstr

The number of valid error codes is fixed. This commit takes advantage of
that to use a static array for caching instead of a sync.Map.

Author: charlievieth

error.go

@@ -14,7 +14,6 @@ package sqlite3
 */
 import "C"
 import (
-	"sync"
 	"sync/atomic"
 	"syscall"
 )
@@ -153,38 +152,45 @@ const (
 	ErrWarningAutoIndex       = ErrNoExtended(ErrWarning | 1<<8)
 )
 
-var errStrCache struct {
-	sync.Map               // int => string
-	size     atomic.Uint64 // size of cache
-	intern   sync.Map      // interned error messages: string => string
-}
+// Cached results of sqlite3_errstr for common errors.
+// NB: this needs to be large enough to hold the last
+// common error (currently SQLITE_WARNING).
+var errStrCache [ErrWarning + 1]atomic.Pointer[string]
 
-// errorString returns the result of sqlite3_errstr for result code rv,
+// errorString returns the result of sqlite3_errstr for result code rc,
 // which may be cached.
-func errorString(rv int) string {
-	if v, ok := errStrCache.Load(rv); ok {
-		return v.(string)
+func errorString(rc int) string {
+	// Common error cases.
+	switch rc {
+	case C.SQLITE_ABORT_ROLLBACK:
+		return "abort due to ROLLBACK"
+	case C.SQLITE_ROW:
+		return "another row available"
+	case C.SQLITE_DONE:
+		return "no more rows available"
+	}
+	if uint(rc) < uint(len(errStrCache)) {
+		if p := errStrCache[rc].Load(); p != nil {
+			return *p
+		}
 	}
-	s := C.GoString(C.sqlite3_errstr(C.int(rv)))
+	s := sqlite3ErrStr(rc)
 	if s == "unknown error" {
 		// Prevent the cache from growing unbounded by ignoring invalid
 		// error codes.
 		return "unknown error"
 	}
-	if v, loaded := errStrCache.intern.LoadOrStore(s, s); loaded {
-		s = v.(string)
-	}
-	// NB: This is limit is not precise and we may exceed
-	// it if this function is called concurrently.
-	//
-	// TODO: make the limit less than a power of 2 to reduce
-	// the chance that we resize the map if it is exceeded.
-	if errStrCache.size.Load() < 1024 {
-		if v, loaded := errStrCache.LoadOrStore(rv, s); loaded {
-			s = v.(string)
-		} else {
-			errStrCache.size.Add(1)
-		}
+	if uint(rc) < uint(len(errStrCache)) {
+		// NB: There is a race here that could result in this store
+		// overwriting a valid value, but it is unlikely and fixing
+		// it is not worth the cost of the CAS operation, which can
+		// be expensive on some platforms.
+		errStrCache[rc].Store(&s)
 	}
 	return s
 }
+
+// This function is declared separately so that we can call it from test code.
+func sqlite3ErrStr(rc int) string {
+	return C.GoString(C.sqlite3_errstr(C.int(rc)))
+}

error_test.go

@@ -13,6 +13,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"sort"
 	"testing"
 )
 
@@ -80,7 +81,6 @@ func TestSqlLogicErrors(t *testing.T) {
 	if err.Error() != expectedErr {
 		t.Errorf("Unexpected error: %s, expected %s", err.Error(), expectedErr)
 	}
-
 }
 
 func TestExtendedErrorCodes_ForeignKey(t *testing.T) {
@@ -281,26 +281,42 @@ func TestError_SystemErrno(t *testing.T) {
 	}
 }
 
-func TestErrorStringCacheSize(t *testing.T) {
-	for i := 0; i < 50_000; i++ {
-		_ = errorString(i)
-	}
-	n := 0
-	o := 0
-	errStrCache.Range(func(_, v any) bool {
-		n++
-		return true
-	})
-	errStrCache.intern.Range(func(_, v any) bool {
-		o++
-		return true
-	})
-	if n > 1024 {
-		t.Fatalf("len(errStrCache) should be capped at %d got: %d", 1024, n)
-	}
-	if o > 128 {
-		// If sqlite3 adds a lot of new error messages this value
-		// will need to be increased.
-		t.Errorf("expected errStrMsgCache to be below %d got: %d", 128, o)
+func errorStringTestCodes() []int {
+	// Special cases
+	codes := []int{
+		100,                   // SQLITE_ROW
+		101,                   // SQLITE_DONE
+		int(ErrAbortRollback), // SQLITE_ABORT_ROLLBACK
+	}
+	for code := 0; code < int(ErrWarning)+10; code++ {
+		codes = append(codes, code)
+	}
+	sort.Ints(codes)
+	return codes
+}
+
+func TestErrorString(t *testing.T) {
+	for _, code := range errorStringTestCodes() {
+		got := errorString(code)
+		want := sqlite3ErrStr(code)
+		if got != want {
+			t.Errorf("errorString(%d) = %q; want: %q", code, got, want)
+		}
+	}
+}
+
+// Test that errorString caches valid results.
+func TestErrorStringCaching(t *testing.T) {
+	for _, code := range errorStringTestCodes() {
+		s1 := errorString(code)
+		s2 := errorString(code)
+		if p1, p2 := stringData(s1), stringData(s2); p1 != p2 {
+			kind := "valid"
+			if code > int(ErrWarning) {
+				kind = "invalid"
+			}
+			t.Errorf("Failed to cache %s sqlite3 error code %d (%q): got: %p want: %p",
+				kind, code, s1, p1, p2)
+		}
 	}
 }