Remove a pair of what are arguably unnecessary mutex locks (and often hit). More JNI-internal cleanups.

FossilOrigin-Name: ecf07a0144dc6402b1e0924b1775d99dc465b27aa86a2718cac60a9b4c974312

ext/jni/src/c/sqlite3-jni.c

@@ -637,7 +637,6 @@ struct S3JniGlobalType {
     volatile unsigned nEnvHit;
     volatile unsigned nEnvMiss;
     volatile unsigned nEnvAlloc;
-    volatile unsigned nNphInit;
     volatile unsigned nMutexEnv       /* number of times envCache.mutex was entered for
                                          a S3JniEnv operation. */;
     volatile unsigned nMutexEnv2      /* number of times envCache.mutex was entered */;
@@ -670,26 +669,6 @@ struct S3JniGlobalType {
 static S3JniGlobalType S3JniGlobal = {};
 #define SJG S3JniGlobal
 
-/*
-** Helpers for extracting pointers from jobjects, noting that we rely
-** on the corresponding Java interfaces having already done the
-** type-checking. OBJ must be a jobject referring to a
-** NativePointerHolder<T>, where T matches PtrGet_T. Don't use these
-** in contexts where that's not the case. Note that these aren't
-** type-safe in the strictest sense:
-**
-**   sqlite3 * s = PtrGet_sqlite3_stmt(...)
-**
-** will work, despite the incorrect macro name, so long as the
-** argument is a Java sqlite3 object, as this operation only has void
-** pointers to work with.
-*/
-#define PtrGet_T(T,OBJ) NativePointerHolder_get(OBJ, &S3JniNphRefs.T)
-#define PtrGet_sqlite3(OBJ) PtrGet_T(sqlite3, OBJ)
-#define PtrGet_sqlite3_stmt(OBJ) PtrGet_T(sqlite3_stmt, OBJ)
-#define PtrGet_sqlite3_value(OBJ) PtrGet_T(sqlite3_value, OBJ)
-#define PtrGet_sqlite3_context(OBJ) PtrGet_T(sqlite3_context, OBJ)
-
 /* Increments *p, possibly protected by a mutex. */
 #ifndef SQLITE_JNI_ENABLE_METRICS
 #define s3jni_incr(PTR)
@@ -1263,7 +1242,6 @@ static S3JniNphClass * S3JniGlobal__nph(JNIEnv * const env, S3JniNphRef const* p
           && (pRef->index < (sizeof(S3JniNphRefs) / sizeof(S3JniNphRef))) );
   if( !pNC->pRef ){
     S3JniMutex_Nph_enter;
-    s3jni_incr( &SJG.metrics.nNphInit );
     if( !pNC->pRef ){
       jclass const klazz = (*env)->FindClass(env, pRef->zName);
       S3JniExceptionIsFatal("FindClass() unexpectedly threw");
@@ -1297,7 +1275,6 @@ static jfieldID s3jni_nphop_field(JNIEnv * const env, S3JniNphRef const* pRef){
 
   if( !pNC->fidValue ){
     S3JniMutex_Nph_enter;
-    s3jni_incr( &SJG.metrics.nNphInit );
     if( !pNC->fidValue ){
       pNC->fidValue = (*env)->GetFieldID(env, pNC->klazz,
                                          pRef->zMember, pRef->zTypeSig);
@@ -1315,13 +1292,12 @@ static jfieldID s3jni_nphop_field(JNIEnv * const env, S3JniNphRef const* pRef){
 ** zClassName must be a static string so we can use its address
 ** as a cache key.
 */
-static void NativePointerHolder__set(JNIEnv * env, S3JniNphRef const* pRef,
+static void NativePointerHolder__set(JNIEnv * const env, S3JniNphRef const* pRef,
                                      jobject ppOut, const void * p){
   jfieldID const fid = s3jni_nphop_field(env, pRef);
 
-  S3JniMutex_Nph_enter;
+  assert( ppOut );
   (*env)->SetLongField(env, ppOut, fid, (jlong)p);
-  S3JniMutex_Nph_leave;
   S3JniExceptionIsFatal("Could not set NativePointerHolder.nativePointer.");
 }
 
@@ -1338,9 +1314,7 @@ static void * NativePointerHolder__get(JNIEnv * env, jobject pObj,
   void * rv = 0;
   if( pObj ){
     jfieldID const fid = s3jni_nphop_field(env, pRef);
-    S3JniMutex_Nph_enter;
     rv = (void*)(*env)->GetLongField(env, pObj, fid);
-    S3JniMutex_Nph_leave;
     S3JniExceptionIsFatal("Cannot fetch NativePointerHolder.nativePointer.");
   }
   return rv;
@@ -1349,6 +1323,43 @@ static void * NativePointerHolder__get(JNIEnv * env, jobject pObj,
 #define NativePointerHolder_get(JOBJ,NPHREF) \
   NativePointerHolder__get(env, (JOBJ), (NPHREF))
 
+/*
+** Helpers for extracting pointers from jobjects, noting that we rely
+** on the corresponding Java interfaces having already done the
+** type-checking. OBJ must be a jobject referring to a
+** NativePointerHolder<T>, where T matches PtrGet_T. Don't use these
+** in contexts where that's not the case. Note that these aren't
+** type-safe in the strictest sense:
+**
+**   sqlite3 * s = PtrGet_sqlite3_stmt(...)
+**
+** will work, despite the incorrect macro name, so long as the
+** argument is a Java sqlite3 object, as this operation only has void
+** pointers to work with.
+*/
+#define PtrGet_T(T,OBJ) NativePointerHolder_get(OBJ, &S3JniNphRefs.T)
+#define PtrGet_sqlite3(OBJ) PtrGet_T(sqlite3, OBJ)
+#define PtrGet_sqlite3_stmt(OBJ) PtrGet_T(sqlite3_stmt, OBJ)
+#define PtrGet_sqlite3_value(OBJ) PtrGet_T(sqlite3_value, OBJ)
+#define PtrGet_sqlite3_context(OBJ) PtrGet_T(sqlite3_context, OBJ)
+
+#if 0
+/*
+** Enters the S3JniDb mutex and PtrGet_sqlite3()'s jObj. If that's
+** NULL then it leaves the mutex, else the mutex is still entered
+** when this returns and the caller is obligated to leave it.
+*/
+static sqlite3* PtrGet__sqlite3_lock(JNIEnv * const env, jobject jObj){
+  sqlite3 *rv;
+  S3JniMutex_S3JniDb_enter;
+  rv = PtrGet_sqlite3(jObj);
+  if( !rv ){ S3JniMutex_S3JniDb_leave; }
+  return rv;
+}
+#undef PtrGet_sqlite3
+#define PtrGet_sqlite3(JOBJ) PtrGet__sqlite3_lock(env, (JOBJ))
+#endif
+
 /*
 ** Extracts the new S3JniDb instance from the free-list, or allocates
 ** one if needed, associats it with pDb, and returns.  Returns NULL on
@@ -1483,8 +1494,7 @@ static int S3JniAutoExtension_init(JNIEnv *const env,
 */
 static void OutputPointer_set_Int32(JNIEnv * const env, jobject const jOut,
                                     int v){
-  (*env)->SetIntField(env, jOut,
+  (*env)->SetIntField(env, jOut, s3jni_nphop_field(
-                      s3jni_nphop_field(
                         env, &S3JniNphRefs.OutputPointer_Int32
                       ), (jint)v);
   S3JniExceptionIsFatal("Cannot set OutputPointer.Int32.value");
@@ -1496,8 +1506,7 @@ static void OutputPointer_set_Int32(JNIEnv * const env, jobject const jOut,
 */
 static void OutputPointer_set_Int64(JNIEnv * const env, jobject const jOut,
                                     jlong v){
-  (*env)->SetLongField(env, jOut,
+  (*env)->SetLongField(env, jOut, s3jni_nphop_field(
-                       s3jni_nphop_field(
                          env, &S3JniNphRefs.OutputPointer_Int64
                        ), v);
   S3JniExceptionIsFatal("Cannot set OutputPointer.Int64.value");
@@ -1583,7 +1592,7 @@ static int encodingTypeIsValid(int eTextRep){
 /*
 ** Proxy for Java-side CollationCallback.xCompare() callbacks.
 */
-static int CollationState_xCompare(void *pArg, int nLhs, const void *lhs,
+static int CollationCallback_xCompare(void *pArg, int nLhs, const void *lhs,
                                    int nRhs, const void *rhs){
   S3JniDb * const ps = pArg;
   S3JniDeclLocal_env;
@@ -1611,8 +1620,8 @@ static int CollationState_xCompare(void *pArg, int nLhs, const void *lhs,
   return (int)rc;
 }
 
-/* Collation finalizer for use by the sqlite3 internals. */
+/* CollationCallback finalizer for use by the sqlite3 internals. */
-static void CollationState_xDestroy(void *pArg){
+static void CollationCallback_xDestroy(void *pArg){
   S3JniDb * const ps = pArg;
   S3JniDeclLocal_env;
 
@@ -1654,7 +1663,6 @@ static jobject new_NativePointerHolder_object(JNIEnv * const env, S3JniNphRef co
   S3JniNphClass * const pNC = S3JniGlobal_nph(pRef);
   if( !pNC->midCtor ){
     S3JniMutex_Nph_enter;
-    s3jni_incr( &SJG.metrics.nNphInit );
     if( !pNC->midCtor ){
       pNC->midCtor = (*env)->GetMethodID(env, pNC->klazz, "<init>", "()V");
       S3JniExceptionIsFatal("Cannot find constructor for class.");
@@ -2404,9 +2412,9 @@ static jint s3jni_close_db(JNIEnv * const env, jobject jDb, int version){
       ? (jint)sqlite3_close(ps->pDb)
       : (jint)sqlite3_close_v2(ps->pDb);
     if( 0==rc ){
+      NativePointerHolder_set(&S3JniNphRefs.sqlite3, jDb, 0);
       S3JniDb_set_aside(ps)
         /* MUST come after close() because of ps->trace. */;
-      NativePointerHolder_set(&S3JniNphRefs.sqlite3, jDb, 0);
     }
   }
 #else
@@ -2825,8 +2833,8 @@ S3JniApi(sqlite3_create_collation() sqlite3_create_collation_v2(),
     if( zName ){
       S3JniMutex_S3JniDb_enter;
       rc = sqlite3_create_collation_v2(ps->pDb, zName, (int)eTextRep,
-                                       ps, CollationState_xCompare,
+                                       ps, CollationCallback_xCompare,
-                                       CollationState_xDestroy);
+                                       CollationCallback_xDestroy);
       sqlite3_free(zName);
       if( 0==rc ){
         S3JniHook_unref( &ps->hooks.collation, 1 );
@@ -3056,8 +3064,8 @@ S3JniApi(sqlite3_errstr(),jstring,1errstr)(
 S3JniApi(sqlite3_expanded_sql(),jstring,1expanded_1sql)(
   JniArgsEnvClass, jobject jpStmt
 ){
-  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt);
   jstring rv = 0;
+  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt);
   if( pStmt ){
     char * zSql = sqlite3_expanded_sql(pStmt);
     s3jni_oom_fatal(zSql);
@@ -3072,7 +3080,8 @@ S3JniApi(sqlite3_expanded_sql(),jstring,1expanded_1sql)(
 S3JniApi(sqlite3_extended_result_codes(),jboolean,1extended_1result_1codes)(
   JniArgsEnvClass, jobject jpDb, jboolean onoff
 ){
-  int const rc = sqlite3_extended_result_codes(PtrGet_sqlite3(jpDb), onoff ? 1 : 0);
+  sqlite3 * const pDb = PtrGet_sqlite3(jpDb);
+  int const rc = pDb ? sqlite3_extended_result_codes(pDb, onoff ? 1 : 0) : 0;
   return rc ? JNI_TRUE : JNI_FALSE;
 }
 
@@ -3098,7 +3107,9 @@ S3JniApi(sqlite3_interrupt(),void,1interrupt)(
   JniArgsEnvClass, jobject jpDb
 ){
   sqlite3 * const pDb = PtrGet_sqlite3(jpDb);
-  if( pDb ) sqlite3_interrupt(pDb);
+  if( pDb ){
+    sqlite3_interrupt(pDb);
+  }
 }
 
 S3JniApi(sqlite3_is_interrupted(),jboolean,1is_1interrupted)(
@@ -3129,7 +3140,12 @@ JniDecl(jboolean,1java_1uncache_1thread)(JniArgsEnvClass){
 S3JniApi(sqlite3_last_insert_rowid(),jlong,1last_1insert_1rowid)(
   JniArgsEnvClass, jobject jpDb
 ){
-  return (jlong)sqlite3_last_insert_rowid(PtrGet_sqlite3(jpDb));
+  jlong rc = 0;
+  sqlite3 * const pDb = PtrGet_sqlite3(jpDb);
+  if( pDb ){
+    rc = (jlong)sqlite3_last_insert_rowid(pDb);
+  }
+  return rc;
 }
 
 /* Pre-open() code common to sqlite3_open[_v2](). */
@@ -4254,15 +4270,13 @@ JniDecl(void,1jni_1internal_1details)(JniArgsEnvClass){
   printf("Mutex entry:"
          "\n\tglobal       = %u"
          "\n\tenv          = %u"
-         "\n\tnph          = %u (%u for S3JniNphClass init, rest for "
+         "\n\tnph          = %u for S3JniNphClass init"
-                                "native pointer access)"
          "\n\tperDb        = %u"
          "\n\tautoExt list = %u"
-         "\n\tS3JniUdf free-list = %u"
+         "\n\tS3JniUdf     = %u (free-list)"
          "\n\tmetrics      = %u\n",
          SJG.metrics.nMutexGlobal, SJG.metrics.nMutexEnv,
-         SJG.metrics.nMutexEnv2, SJG.metrics.nNphInit,
+         SJG.metrics.nMutexEnv2, SJG.metrics.nMutexPerDb,
-         SJG.metrics.nMutexPerDb,
          SJG.metrics.nMutexAutoExt, SJG.metrics.nMutexUdf,
          SJG.metrics.nMetrics);
   puts("Allocs:");

manifest

@@ -1,5 +1,5 @@
-C Update\stool/mkctimec.tcl\sto\saccount\sfor\s[fe7365254d343e].
+C Remove\sa\spair\sof\swhat\sare\sarguably\sunnecessary\smutex\slocks\s(and\soften\shit).\sMore\sJNI-internal\scleanups.
-D 2023-08-28T16:05:19.091
+D 2023-08-28T16:22:31.875
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -236,7 +236,7 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3
 F ext/jni/GNUmakefile 374873bf6d2cd6ceafb458e28b59140dbb074f01f7adddf7e15a3ee3daf44551
 F ext/jni/README.md 1332b1fa27918bd5d9ca2d0d4f3ac3a6ab86b9e3699dc5bfe32904a027f3d2a9
 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa
-F ext/jni/src/c/sqlite3-jni.c 0b76d7947e474aa30f6636f42a4e00eb48ccad6f920f1d2a1ee2d9540709c578
+F ext/jni/src/c/sqlite3-jni.c c92a764a728a3fcd34882defb7517c47af22c9f7fe184113015cdab9fe401c21
 F ext/jni/src/c/sqlite3-jni.h 12e1a5ef5ee1795dc22577c285b4518dfd8aa4af45757f6cb81a555d967bf201
 F ext/jni/src/org/sqlite/jni/AbstractCollationCallback.java 95e88ba04f4aac51ffec65693e878e234088b2f21b387f4e4285c8b72b33e436
 F ext/jni/src/org/sqlite/jni/AggregateFunction.java 7312486bc65fecdb91753c0a4515799194e031f45edbe16a6373cea18f404dc4
@@ -2106,8 +2106,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P fe7365254d343ed03e11a4e9cad7f0e5d5182b9220c6fde6d30e434ebdaba2af
+P 349aac7e8d513bc420e8948b84cf715e454443439fdcd5aff0f2b13815a9cbb3
-R b00a271c5a6f2785833f80aa9f660281
+R 3d489af3468d8a7b527ca6166e876cc6
 U stephan
-Z 28d128e867bc57e370acf90e968cbcdb
+Z a77195f4742188b139cedf649b9d3b8f
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-349aac7e8d513bc420e8948b84cf715e454443439fdcd5aff0f2b13815a9cbb3
+ecf07a0144dc6402b1e0924b1775d99dc465b27aa86a2718cac60a9b4c974312