From: stephan Date: Mon, 28 Aug 2023 21:27:32 +0000 (+0000) Subject: Improve threadability of the JNI collation-related bindings and add infrastructure... X-Git-Tag: version-3.44.0~247 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fa23b4fc61ff45643e689849da00f7d62cf0cf55;p=thirdparty%2Fsqlite.git Improve threadability of the JNI collation-related bindings and add infrastructure for similar cases. FossilOrigin-Name: f02dad66b965b9e3c504001e9603af8f74977f151bede9db369f88e86a4aeb00 --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index dfa929ce90..9eada125a1 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -404,6 +404,8 @@ struct S3JniHook{ ** jObj's type */; /* We lookup the jObj.xDestroy() method as-needed for contexts which ** have custom finalizers. */ + jobject jExtra /* Global ref to a per-hook-type value */; + S3JniHook * pNextFree /* Next entry in S3Global.hooks.aFree */; }; /* For clean bitwise-copy init of local instances. */ static const S3JniHook S3JniHook_empty = {0,0}; @@ -627,7 +629,9 @@ struct S3JniGlobalType { #endif #ifdef SQLITE_ENABLE_SQLLOG struct { - S3JniHook sqllog /* sqlite3_config(SQLITE_CONFIG_SQLLOG) callback */; + S3JniHook sqllog /* sqlite3_config(SQLITE_CONFIG_SQLLOG) callback */; + S3JniHook * aFree /* free-item list, for recycling. Guarded by + the global mutex. */; } hooks; #endif #ifdef SQLITE_JNI_ENABLE_METRICS @@ -649,6 +653,8 @@ struct S3JniGlobalType { volatile unsigned nPdbRecycled /* Number of S3JniDb reused. */; volatile unsigned nUdfAlloc /* Number of S3JniUdf alloced. */; volatile unsigned nUdfRecycled /* Number of S3JniUdf reused. */; + volatile unsigned nHookAlloc /* Number of S3JniHook alloced. */; + volatile unsigned nHookRecycled /* Number of S3JniHook reused. */; struct { /* Number of calls for each type of UDF callback. */ volatile unsigned nFunc; @@ -1077,16 +1083,16 @@ static void s3jni__call_xDestroy(JNIEnv * const env, jobject jObj){ /* ** Internal helper for many hook callback impls. Locks the S3JniDb -** mutex, makes a copy of src into dest, with one change: if src->jObj -** is not NULL then dest->jObj will be a new LOCAL ref to src->jObj -** instead of a copy of the prior GLOBAL ref. Then it unlocks the -** mutex. +** mutex, makes a copy of src into dest, with one differs: if +** src->jObj or src->jExtra are not NULL then dest will be a new LOCAL +** ref to it instead of a copy of the prior GLOBAL ref. Then it +** unlocks the mutex. ** ** If dest->jObj is not NULL when this returns then the caller is ** obligated to eventually free the new ref by passing *dest to ** S3JniHook_localundup(). The dest pointer must NOT be passed to -** S3JniHook_unref(), as that routine assumes that dest->jObj is a -** GLOBAL ref (it's illegal to try to unref the wrong ref type).. +** S3JniHook_unref(), as that routine assumes that dest->jObj/jExtra +** are GLOBAL refs (it's illegal to try to unref the wrong ref type). ** ** Background: when running a hook we need a call-local copy lest ** another thread modify the hook while we're running it. That copy @@ -1096,11 +1102,18 @@ static void S3JniHook__localdup( JNIEnv * const env, S3JniHook const * const src S3JniHook * const dest ){ S3JniMutex_S3JniDb_enter; *dest = *src; - if(dest->jObj) dest->jObj = S3JniRefLocal(dest->jObj); + if(src->jObj) dest->jObj = S3JniRefLocal(src->jObj); + if(src->jExtra) dest->jExtra = S3JniRefLocal(src->jExtra); S3JniMutex_S3JniDb_leave; } #define S3JniHook_localdup(src,dest) S3JniHook__localdup(env,src,dest) -#define S3JniHook_localundup(HOOK) S3JniUnrefLocal(HOOK.jObj) + +static void S3JniHook__localundup( JNIEnv * const env, S3JniHook * const h ){ + S3JniUnrefLocal(h->jObj); + S3JniUnrefLocal(h->jExtra); + memset(h, 0, sizeof(*h)); +} +#define S3JniHook_localundup(HOOK) S3JniHook__localundup(env, &(HOOK)) /* ** Removes any Java references from s and clears its state. If @@ -1110,18 +1123,61 @@ static void S3JniHook__localdup( JNIEnv * const env, S3JniHook const * const src ** references. */ static void S3JniHook__unref(JNIEnv * const env, S3JniHook * const s, - int doXDestroy){ + int doXDestroy){ if( s->jObj ){ if( doXDestroy ){ s3jni_call_xDestroy(s->jObj); } S3JniUnrefGlobal(s->jObj); + //S3JniUnrefGlobal(s->jExtra); } memset(s, 0, sizeof(*s)); } #define S3JniHook_unref(hook,doDestroy) \ S3JniHook__unref(env, (hook), (doDestroy)) +static S3JniHook *S3JniHook__alloc(JNIEnv * const env){ + S3JniHook * p = 0; + S3JniMutex_Global_enter; + if( SJG.hooks.aFree ){ + p = SJG.hooks.aFree; + SJG.hooks.aFree = p->pNextFree; + p->pNextFree = 0; + s3jni_incr(&SJG.metrics.nHookRecycled); + } + S3JniMutex_Global_leave; + if( 0==p ){ + p = s3jni_malloc(sizeof(S3JniHook)); + if( p ){ + s3jni_incr(&SJG.metrics.nHookAlloc); + } + } + if( p ){ + *p = S3JniHook_empty; + } + return p; +} + +#define S3JniHook_alloc() S3JniHook__alloc(env) + +/* +** The rightful fate of all results from S3JniHook_alloc(). doXDestroy +** is passed on as-is to S3JniHook_unref(). +*/ +static void S3JniHook__free(JNIEnv * const env, S3JniHook * const p, + int doXDestroy){ + if(p){ + assert( !p->pNextFree ); + S3JniHook_unref(p, doXDestroy); + S3JniMutex_Global_enter; + p->pNextFree = SJG.hooks.aFree; + SJG.hooks.aFree = p; + S3JniMutex_Global_leave; + } +} +#define S3JniHook_free(hook,doXDestroy) S3JniHook__free(env, hook, doXDestroy) + + /* ** Clears all of s's state. Requires that that the caller has locked ** S3JniGlobal.perDb.mutex. Make sure to do anything needed with @@ -1596,46 +1652,6 @@ static int encodingTypeIsValid(int eTextRep){ } } -/** - State for CollationCallbacks. -*/ -typedef S3JniHook S3JniCollationCallback; - -/* -** Proxy for Java-side CollationCallback.xCompare() callbacks. -*/ -static int CollationCallback_xCompare(void *pArg, int nLhs, const void *lhs, - int nRhs, const void *rhs){ - S3JniCollationCallback * const pCC = pArg; - S3JniDeclLocal_env; - jint rc = 0; - if( pCC->jObj ){ - jbyteArray jbaLhs = s3jni_new_jbyteArray(lhs, (jint)nLhs); - jbyteArray jbaRhs = jbaLhs - ? s3jni_new_jbyteArray(rhs, (jint)nRhs) : 0; - if( !jbaRhs ){ - S3JniUnrefLocal(jbaLhs); - /* We have no recovery strategy here. */ - s3jni_oom_check( jbaRhs ); - return 0; - } - rc = (*env)->CallIntMethod(env, pCC->jObj, pCC->midCallback, - jbaLhs, jbaRhs); - S3JniExceptionIgnore; - S3JniUnrefLocal(jbaLhs); - S3JniUnrefLocal(jbaRhs); - } - return (int)rc; -} - -/* CollationCallback finalizer for use by the sqlite3 internals. */ -static void CollationCallback_xDestroy(void *pArg){ - S3JniCollationCallback * const pCC = pArg; - S3JniDeclLocal_env; - S3JniHook_unref(pCC, 1); - sqlite3_free(pCC); -} - /* For use with sqlite3_result/value_pointer() */ #define ResultJavaValuePtrStr "org.sqlite.jni.ResultJavaVal" @@ -2487,33 +2503,30 @@ static unsigned int s3jni_utf16_strlen(void const * z){ return i; } +/* Descriptive alias for use with sqlite3_create_collation(). */ +typedef S3JniHook S3JniCollationNeeded; + /* Central C-to-Java sqlite3_collation_needed16() hook impl. */ static void s3jni_collation_needed_impl16(void *pState, sqlite3 *pDb, int eTextRep, const void * z16Name){ - S3JniDb * const ps = pState; + S3JniCollationNeeded * const pHook = pState; S3JniDeclLocal_env; S3JniHook hook; - S3JniHook_localdup(&ps->hooks.collationNeeded, &hook ); + S3JniHook_localdup(pHook, &hook); if( hook.jObj ){ unsigned int const nName = s3jni_utf16_strlen(z16Name); jstring jName = (*env)->NewString(env, (jchar const *)z16Name, nName); + s3jni_oom_check( jName ); + assert( hook.jExtra ); S3JniIfThrew{ S3JniExceptionClear; - }else{ - jobject jDb; - S3JniMutex_S3JniDb_enter; - jDb = ps->jDb ? S3JniRefLocal(ps->jDb) : 0; - S3JniMutex_S3JniDb_leave; - if( jDb ){ - (*env)->CallVoidMethod(env, hook.jObj, hook.midCallback, - jDb, (jint)eTextRep, jName); - S3JniIfThrew{ - S3JniExceptionWarnCallbackThrew("sqlite3_collation_needed() callback"); - s3jni_db_exception(ps, 0, "sqlite3_collation_needed() callback threw"); - } - S3JniUnrefLocal(jDb); + }else if( hook.jExtra ){ + (*env)->CallVoidMethod(env, hook.jObj, hook.midCallback, + hook.jExtra, (jint)eTextRep, jName); + S3JniIfThrew{ + S3JniExceptionWarnCallbackThrew("sqlite3_collation_needed() callback"); } } S3JniUnrefLocal(jName); @@ -2525,7 +2538,7 @@ S3JniApi(sqlite3_collation_needed(),jint,1collation_1needed)( JniArgsEnvClass, jobject jDb, jobject jHook ){ S3JniDb * ps; - S3JniHook * pHook; + S3JniCollationNeeded * pHook; int rc = 0; S3JniMutex_S3JniDb_enter; @@ -2554,11 +2567,12 @@ S3JniApi(sqlite3_collation_needed(),jint,1collation_1needed)( "Cannot not find matching call() in " "CollationNeededCallback object."); }else{ - rc = sqlite3_collation_needed16(ps->pDb, ps, s3jni_collation_needed_impl16); + rc = sqlite3_collation_needed16(ps->pDb, pHook, s3jni_collation_needed_impl16); if( 0==rc ){ S3JniHook_unref(pHook, 0); pHook->midCallback = xCallback; pHook->jObj = S3JniRefGlobal(jHook); + pHook->jExtra = S3JniRefGlobal(jDb); } } } @@ -2846,43 +2860,86 @@ S3JniApi(sqlite3_context_db_handle(),jobject,1context_1db_1handle)( return ps ? ps->jDb : 0; } +/** + State for CollationCallbacks. +*/ +typedef S3JniHook S3JniCollationCallback; + +/* +** Proxy for Java-side CollationCallback.xCompare() callbacks. +*/ +static int CollationCallback_xCompare(void *pArg, int nLhs, const void *lhs, + int nRhs, const void *rhs){ + S3JniCollationCallback * const pCC = pArg; + S3JniDeclLocal_env; + jint rc = 0; + if( pCC->jObj ){ + jbyteArray jbaLhs = s3jni_new_jbyteArray(lhs, (jint)nLhs); + jbyteArray jbaRhs = jbaLhs + ? s3jni_new_jbyteArray(rhs, (jint)nRhs) : 0; + if( !jbaRhs ){ + S3JniUnrefLocal(jbaLhs); + /* We have no recovery strategy here. */ + s3jni_oom_check( jbaRhs ); + return 0; + } + rc = (*env)->CallIntMethod(env, pCC->jObj, pCC->midCallback, + jbaLhs, jbaRhs); + S3JniExceptionIgnore; + S3JniUnrefLocal(jbaLhs); + S3JniUnrefLocal(jbaRhs); + } + return (int)rc; +} + +/* CollationCallback finalizer for use by the sqlite3 internals. */ +static void CollationCallback_xDestroy(void *pArg){ + S3JniCollationCallback * const pCC = pArg; + S3JniDeclLocal_env; + S3JniHook_free(pCC, 1); +} + S3JniApi(sqlite3_create_collation() sqlite3_create_collation_v2(), jint,1create_1collation )(JniArgsEnvClass, jobject jDb, jstring name, jint eTextRep, jobject oCollation){ int rc; - jclass klazz; - S3JniDb * const ps = S3JniDb_from_java(jDb); - jmethodID midCallback; + S3JniDb * ps; - if( !ps ) return SQLITE_MISUSE; - klazz = (*env)->GetObjectClass(env, oCollation); - midCallback = (*env)->GetMethodID(env, klazz, "call", "([B[B)I"); - S3JniUnrefLocal(klazz); - S3JniIfThrew{ - rc = s3jni_db_error(ps->pDb, SQLITE_ERROR, - "Could not get call() method from " - "CollationCallback object."); + S3JniMutex_S3JniDb_enter; + ps = S3JniDb_from_java_unlocked(jDb); + if( !ps ){ + rc = SQLITE_MISUSE; }else{ - char * const zName = s3jni_jstring_to_utf8( name, 0); - S3JniCollationCallback * const pCC = - zName ? s3jni_malloc(sizeof(S3JniCollationCallback)) : 0; - if( pCC ){ - memset( pCC, 0, sizeof(*pCC) ); - rc = sqlite3_create_collation_v2(ps->pDb, zName, (int)eTextRep, - pCC, CollationCallback_xCompare, - CollationCallback_xDestroy); - if( 0==rc ){ - pCC->midCallback = midCallback; - pCC->jObj = S3JniRefGlobal(oCollation); + jclass const klazz = (*env)->GetObjectClass(env, oCollation); + jmethodID const midCallback = + (*env)->GetMethodID(env, klazz, "call", "([B[B)I"); + S3JniUnrefLocal(klazz); + S3JniIfThrew{ + rc = s3jni_db_error(ps->pDb, SQLITE_ERROR, + "Could not get call() method from " + "CollationCallback object."); + }else{ + char * const zName = s3jni_jstring_to_utf8( name, 0); + S3JniCollationCallback * const pCC = + zName ? S3JniHook_alloc() : 0; + if( pCC ){ + rc = sqlite3_create_collation_v2(ps->pDb, zName, (int)eTextRep, + pCC, CollationCallback_xCompare, + CollationCallback_xDestroy); + if( 0==rc ){ + pCC->midCallback = midCallback; + pCC->jObj = S3JniRefGlobal(oCollation); + }else{ + CollationCallback_xDestroy(pCC); + } }else{ - CollationCallback_xDestroy(pCC); + rc = SQLITE_NOMEM; } - }else{ - rc = SQLITE_NOMEM; + sqlite3_free(zName); } - sqlite3_free(zName); } + S3JniMutex_S3JniDb_leave; return (jint)rc; } @@ -4323,6 +4380,10 @@ JniDecl(void,1jni_1internal_1details)(JniArgsEnvClass){ SJG.metrics.nUdfAlloc, (unsigned) sizeof(S3JniUdf), (unsigned)(SJG.metrics.nUdfAlloc * sizeof(S3JniUdf)), SJG.metrics.nUdfRecycled); + printf("\tS3JniHook: %u alloced (*%u = %u bytes), %u recycled\n", + SJG.metrics.nHookAlloc, (unsigned) sizeof(S3JniHook), + (unsigned)(SJG.metrics.nHookAlloc * sizeof(S3JniHook)), + SJG.metrics.nHookRecycled); printf("\tS3JniEnv: %u alloced (*%u = %u bytes)\n", SJG.metrics.nEnvAlloc, (unsigned) sizeof(S3JniEnv), (unsigned)(SJG.metrics.nEnvAlloc * sizeof(S3JniEnv))); diff --git a/manifest b/manifest index ae37df779d..b7c342d6f0 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Correct\sJNI\smapping\sof\scollations\sto\sbe\s1-db-to-many-collations. -D 2023-08-28T20:21:56.713 +C Improve\sthreadability\sof\sthe\sJNI\scollation-related\sbindings\sand\sadd\sinfrastructure\sfor\ssimilar\scases. +D 2023-08-28T21:27:32.287 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -237,7 +237,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 3c16035f6d604c17e50a6477b7309ca8885c54cd8a9c85e8c36d2f8b918d71e7 +F ext/jni/src/c/sqlite3-jni.c 48128298ecdd99ddd4f3068c73cc2548899c70cea8a1ddc9e651248b815854db 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 @@ -2107,8 +2107,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 ceeabe9f8b31a30c65147fd270b92d43c7842247548cee9de165113991f6c2cf -R 68323f68f37601aa48cb5f2c61fd0c0a +P b927b0f5a68684b0a9799396d153bf1e2306351e8039c2bacb3d5b2056a0634f +R f4a0af7c14adbc7eb10004ae27d12901 U stephan -Z 6560ea32a29f03de2be1f2fe7d94526d +Z 0bf576aa532ec9693d521343b6887da2 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 3ef7700236..a2681cfa81 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -b927b0f5a68684b0a9799396d153bf1e2306351e8039c2bacb3d5b2056a0634f \ No newline at end of file +f02dad66b965b9e3c504001e9603af8f74977f151bede9db369f88e86a4aeb00 \ No newline at end of file