Initial infrastructure for adding a mode to the OPFS VFS which causes implicit locks...

FossilOrigin-Name: c5b7a9715a13b696ab3ee965aa0a310f59b65f07cecd72faa2e3504bfd8eb632

diff --git a/ext/wasm/api/sqlite3-api-oo1.js b/ext/wasm/api/sqlite3-api-oo1.js
index 02ce9c0ced7b5f19a556d2e7fdd186744f27beed..bc32ec1067cc974fb07debf97cd1cc49fa7fc60d 100644 (file)
--- a/ext/wasm/api/sqlite3-api-oo1.js
+++ b/ext/wasm/api/sqlite3-api-oo1.js
@@ -200,9 +200,8 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
   */
   dbCtorHelper.normalizeArgs = function(filename=':memory:',flags = 'c',vfs = null){
     const arg = {};
-    if(1===arguments.length && 'object'===typeof arguments[0]){
-      const x = arguments[0];
-      Object.keys(x).forEach((k)=>arg[k] = x[k]);
+    if(1===arguments.length && arguments[0] && 'object'===typeof arguments[0]){
+      Object.assign(arg, arguments[0]);
       if(undefined===arg.flags) arg.flags = 'c';
       if(undefined===arg.vfs) arg.vfs = null;
       if(undefined===arg.filename) arg.filename = ':memory:';

diff --git a/ext/wasm/api/sqlite3-api-opfs.js b/ext/wasm/api/sqlite3-api-opfs.js
index deb4c923ab8f50e7977b26c4a8c6e34f8cba6875..53f68a1d587b723e00805ebab53de5bae01e2b53 100644 (file)
--- a/ext/wasm/api/sqlite3-api-opfs.js
+++ b/ext/wasm/api/sqlite3-api-opfs.js
@@ -348,6 +348,7 @@ const installOpfsVfs = function callee(options){
       'SQLITE_NOTFOUND',
       'SQLITE_OPEN_CREATE',
       'SQLITE_OPEN_DELETEONCLOSE',
+      'SQLITE_OPEN_MAIN_DB',
       'SQLITE_OPEN_READONLY'
     ].forEach((k)=>{
       if(undefined === (state.sq3Codes[k] = capi[k])){

diff --git a/ext/wasm/api/sqlite3-opfs-async-proxy.js b/ext/wasm/api/sqlite3-opfs-async-proxy.js
index c208932e1796bb953a8037881c01221772777a71..cbe0549b3e07fbcfad7b0eb7e8a6bbd8868a7f7f 100644 (file)
--- a/ext/wasm/api/sqlite3-opfs-async-proxy.js
+++ b/ext/wasm/api/sqlite3-opfs-async-proxy.js
@@ -105,7 +105,7 @@ metrics.dump = ()=>{
 */
 const __openFiles = Object.create(null);
 /**
-   __autoLocks is a Set of sqlite3_file pointers (integers) which were
+   __implicitLocks is a Set of sqlite3_file pointers (integers) which were
    "auto-locked".  i.e. those for which we obtained a sync access
    handle without an explicit xLock() call. Such locks will be
    released during db connection idle time, whereas a sync access
@@ -117,7 +117,7 @@ const __openFiles = Object.create(null);
    penalty: speedtest1 benchmarks take up to 4x as long. By delaying
    the lock release until idle time, the hit is negligible.
 */
-const __autoLocks = new Set();
+const __implicitLocks = new Set();
 
 /**
    Expects an OPFS file path. It gets resolved, such that ".."
@@ -166,7 +166,7 @@ const closeSyncHandle = async (fh)=>{
     const h = fh.syncHandle;
     delete fh.syncHandle;
     delete fh.xLock;
-    __autoLocks.delete(fh.fid);
+    __implicitLocks.delete(fh.fid);
     return h.close();
   }
 };
@@ -190,10 +190,10 @@ const closeSyncHandleNoThrow = async (fh)=>{
 };
 
 /* Release all auto-locks. */
-const closeAutoLocks = async ()=>{
-  if(__autoLocks.size){
+const releaseImplicitLocks = async ()=>{
+  if(__implicitLocks.size){
     /* Release all auto-locks. */
-    for(const fid of __autoLocks){
+    for(const fid of __implicitLocks){
       const fh = __openFiles[fid];
       await closeSyncHandleNoThrow(fh);
       log("Auto-unlocked",fid,fh.filenameAbs);
@@ -201,6 +201,32 @@ const closeAutoLocks = async ()=>{
   }
 };
 
+/**
+   If true, any routine which implicitly acquires a sync access handle
+   (i.e. an OPFS lock) will release that locks at the end of the call
+   which acquires it. If false, such "autolocks" are not released
+   until the VFS is idle for some brief amount of time.
+
+   The benefit of enabling this is much higher concurrency. The
+   down-side is much-reduced performance (as much as a 4x decrease
+   in speedtest1).
+*/
+state.defaultReleaseImplicitLocks = false;
+
+/**
+   An experiment in improving concurrency by freeing up implicit locks
+   sooner. This is known to impact performance dramatically but it has
+   also shown to improve concurrency considerably.
+
+   If fh.releaseImplicitLocks is truthy and fh is in __implicitLocks,
+   this routine returns closeSyncHandleNoThrow(), else it is a no-op.
+*/
+const releaseImplicitLock = async (fh)=>{
+  if(fh.releaseImplicitLocks && __implicitLocks.has(fh.fid)){
+    return closeSyncHandleNoThrow(fh);
+  }
+};
+
 /**
    An error class specifically for use with getSyncHandle(), the goal
    of which is to eventually be able to distinguish unambiguously
@@ -246,7 +272,7 @@ GetSyncHandleError.convertRc = (e,rc)=>{
    still fails at that point it will give up and propagate the
    exception.
 */
-const getSyncHandle = async (fh)=>{
+const getSyncHandle = async (fh,opName)=>{
   if(!fh.syncHandle){
     const t = performance.now();
     log("Acquiring sync handle for",fh.filenameAbs);
@@ -262,20 +288,21 @@ const getSyncHandle = async (fh)=>{
       }catch(e){
         if(i === maxTries){
           throw new GetSyncHandleError(
-            e, "Error getting sync handle.",maxTries,
+            e, "Error getting sync handle for",opName+"().",maxTries,
             "attempts failed.",fh.filenameAbs
           );
         }
-        warn("Error getting sync handle. Waiting",ms,
+        warn("Error getting sync handle for",opName+"(). Waiting",ms,
              "ms and trying again.",fh.filenameAbs,e);
-        await closeAutoLocks();
+        //await releaseImplicitLocks();
         Atomics.wait(state.sabOPView, state.opIds.retry, 0, ms);
       }
     }
-    log("Got sync handle for",fh.filenameAbs,'in',performance.now() - t,'ms');
+    log("Got",opName+"() sync handle for",fh.filenameAbs,
+        'in',performance.now() - t,'ms');
     if(!fh.xLock){
-      __autoLocks.add(fh.fid);
-      log("Auto-locked",fh.fid,fh.filenameAbs);
+      __implicitLocks.add(fh.fid);
+      log("Auto-locked for",opName+"()",fh.fid,fh.filenameAbs);
     }
   }
   return fh.syncHandle;
@@ -409,7 +436,7 @@ const vfsAsyncImpls = {
   xClose: async function(fid/*sqlite3_file pointer*/){
     const opName = 'xClose';
     mTimeStart(opName);
-    __autoLocks.delete(fid);
+    __implicitLocks.delete(fid);
     const fh = __openFiles[fid];
     let rc = 0;
     wTimeStart(opName);
@@ -474,13 +501,14 @@ const vfsAsyncImpls = {
     wTimeStart('xFileSize');
     try{
       affirmLocked('xFileSize',fh);
-      const sz = await (await getSyncHandle(fh)).getSize();
+      const sz = await (await getSyncHandle(fh,'xFileSize')).getSize();
       state.s11n.serialize(Number(sz));
       rc = 0;
     }catch(e){
       state.s11n.storeException(2,e);
       rc = GetSyncHandleError.convertRc(e,state.sq3Codes.SQLITE_IOERR);
     }
+    await releaseImplicitLock(fh);
     wTimeEnd();
     storeAndNotify('xFileSize', rc);
     mTimeEnd();
@@ -495,8 +523,8 @@ const vfsAsyncImpls = {
     if( !fh.syncHandle ){
       wTimeStart('xLock');
       try {
-        await getSyncHandle(fh);
-        __autoLocks.delete(fid);
+        await getSyncHandle(fh,'xLock');
+        __implicitLocks.delete(fid);
       }catch(e){
         state.s11n.storeException(1,e);
         rc = GetSyncHandleError.convertRc(e,state.sq3Codes.SQLITE_IOERR_LOCK);
@@ -511,7 +539,6 @@ const vfsAsyncImpls = {
                         flags/*SQLITE_OPEN_...*/){
     const opName = 'xOpen';
     mTimeStart(opName);
-    const deleteOnClose = (state.sq3Codes.SQLITE_OPEN_DELETEONCLOSE & flags);
     const create = (state.sq3Codes.SQLITE_OPEN_CREATE & flags);
     wTimeStart('xOpen');
     try{
@@ -526,14 +553,8 @@ const vfsAsyncImpls = {
         return;
       }
       const hFile = await hDir.getFileHandle(filenamePart, {create});
-      /**
-         wa-sqlite, at this point, grabs a SyncAccessHandle and
-         assigns it to the syncHandle prop of the file state
-         object, but only for certain cases and it's unclear why it
-         places that limitation on it.
-      */
       wTimeEnd();
-      __openFiles[fid] = Object.assign(Object.create(null),{
+      const fh = Object.assign(Object.create(null),{
         fid: fid,
         filenameAbs: filename,
         filenamePart: filenamePart,
@@ -542,8 +563,47 @@ const vfsAsyncImpls = {
         sabView: state.sabFileBufView,
         readOnly: create
           ? false : (state.sq3Codes.SQLITE_OPEN_READONLY & flags),
-        deleteOnClose: deleteOnClose
+        deleteOnClose: !!(state.sq3Codes.SQLITE_OPEN_DELETEONCLOSE & flags)
       });
+      fh.releaseImplicitLocks =
+        state.defaultReleaseImplicitLocks
+        /* TODO: check URI flags for "opfs-auto-unlock". First we need to
+           reshape the API a bit to be able to pass those on to here
+           from the other half of the proxy. */;
+      /*if(fh.releaseImplicitLocks){
+        console.warn("releaseImplicitLocks is ON for",fh);
+      }*/
+      if(0 /* this block is modelled after something wa-sqlite
+              does but it leads to horrible contention on journal files. */
+         && (0===(flags & state.sq3Codes.SQLITE_OPEN_MAIN_DB))){
+        /* sqlite does not lock these files, so go ahead and grab an OPFS
+           lock.
+
+           Regarding "immutable": that flag is not _really_ applicable
+           here.  It's intended for use on read-only media. If,
+           however, a file is opened with that flag but changes later
+           (which can happen if we _don't_ grab a sync handle here)
+           then sqlite may misbehave.
+
+           Regarding "nolock": ironically, the nolock flag forces us
+           to lock the file up front. "nolock" tells sqlite to _not_
+           use its locking API, but OPFS requires a lock to perform
+           most of the operations performed in this file. If we don't
+           grab that lock up front, another handle could end up grabbing
+           it and mutating the database out from under our nolocked'd
+           handle. In the interest of preventing corruption, at the cost
+           of decreased concurrency, we have to lock it for the duration
+           of this file handle.
+
+           https://www.sqlite.org/uri.html
+        */
+        fh.xLock = "atOpen"/* Truthy value to keep entry from getting
+                              flagged as auto-locked. String value so
+                              that we can easily distinguish is later
+                              if needed. */;
+        await getSyncHandle(fh,'xOpen');
+      }
+      __openFiles[fid] = fh;
       storeAndNotify(opName, 0);
     }catch(e){
       wTimeEnd();
@@ -560,7 +620,7 @@ const vfsAsyncImpls = {
     try{
       affirmLocked('xRead',fh);
       wTimeStart('xRead');
-      nRead = (await getSyncHandle(fh)).read(
+      nRead = (await getSyncHandle(fh,'xRead')).read(
         fh.sabView.subarray(0, n),
         {at: Number(offset64)}
       );
@@ -575,6 +635,7 @@ const vfsAsyncImpls = {
       state.s11n.storeException(1,e);
       rc = GetSyncHandleError.convertRc(e,state.sq3Codes.SQLITE_IOERR_READ);
     }
+    await releaseImplicitLock(fh);
     storeAndNotify('xRead',rc);
     mTimeEnd();
   },
@@ -603,12 +664,13 @@ const vfsAsyncImpls = {
     try{
       affirmLocked('xTruncate',fh);
       affirmNotRO('xTruncate', fh);
-      await (await getSyncHandle(fh)).truncate(size);
+      await (await getSyncHandle(fh,'xTruncate')).truncate(size);
     }catch(e){
       error("xTruncate():",e,fh);
       state.s11n.storeException(2,e);
       rc = GetSyncHandleError.convertRc(e,state.sq3Codes.SQLITE_IOERR_TRUNCATE);
     }
+    await releaseImplicitLock(fh);
     wTimeEnd();
     storeAndNotify('xTruncate',rc);
     mTimeEnd();
@@ -640,7 +702,7 @@ const vfsAsyncImpls = {
       affirmLocked('xWrite',fh);
       affirmNotRO('xWrite', fh);
       rc = (
-        n === (await getSyncHandle(fh))
+        n === (await getSyncHandle(fh,'xWrite'))
           .write(fh.sabView.subarray(0, n),
                  {at: Number(offset64)})
       ) ? 0 : state.sq3Codes.SQLITE_IOERR_WRITE;
@@ -649,6 +711,7 @@ const vfsAsyncImpls = {
       state.s11n.storeException(1,e);
       rc = GetSyncHandleError.convertRc(e,state.sq3Codes.SQLITE_IOERR_WRITE);
     }
+    await releaseImplicitLock(fh);
     wTimeEnd();
     storeAndNotify('xWrite',rc);
     mTimeEnd();
@@ -783,7 +846,7 @@ const waitLoop = async function f(){
       if('timed-out'===Atomics.wait(
         state.sabOPView, state.opIds.whichOp, 0, waitTime
       )){
-        await closeAutoLocks();
+        await releaseImplicitLocks();
         continue;
       }
       const opId = Atomics.load(state.sabOPView, state.opIds.whichOp);

diff --git a/ext/wasm/tests/opfs/concurrency/worker.js b/ext/wasm/tests/opfs/concurrency/worker.js
index 19b0a068e7a2d57e77d63d97ecc6608f39fb557d..9b4898f4c35f5b5b850a6ad110bc019b51519737 100644 (file)
--- a/ext/wasm/tests/opfs/concurrency/worker.js
+++ b/ext/wasm/tests/opfs/concurrency/worker.js
@@ -43,7 +43,11 @@ self.sqlite3InitModule().then(async function(sqlite3){
     }
   };
   const run = async function(){
-    db = new sqlite3.opfs.OpfsDb(dbName,'c');
+    db = new sqlite3.oo1.DB({
+      filename: 'file:'+dbName,
+      flags: 'c',
+      vfs: 'opfs'
+    });
     sqlite3.capi.sqlite3_busy_timeout(db.pointer, 5000);
     db.transaction((db)=>{
       db.exec([

diff --git a/manifest b/manifest
index 3ecb2811092e476d766594cf78fe18cc13eeeb8a..d6e43cfa43dc859237c9ced1694b1749b2e197cd 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Update\sMakefile.in\sto\sinclude\snew\starget\s"sqlite3r.c".\sFor\sgenerating\s"sqlite3r.c"\sand\s"sqlite3r.h",\sversions\sof\sthe\samalgamation\sthat\sinclude\sthe\srecover\sextension.\sTo\sbuild\sthe\sshell\stool\sagainst\sthese\sfiles,\sadd\s-DSQLITE_HAVE_SQLITE3R.
-D 2022-11-23T16:08:49.765
+C Initial\sinfrastructure\sfor\sadding\sa\smode\sto\sthe\sOPFS\sVFS\swhich\scauses\simplicit\slocks\sto\sbe\sreleased\sASAP,\swhich\sincreases\sconcurrency\sat\sthe\scost\sof\sperformance.
+D 2022-11-23T16:39:07.866
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -501,12 +501,12 @@ F ext/wasm/api/post-js-header.js d6ab3dfef4a06960d28a7eaa338d4e2a1a5981e9b387181
 F ext/wasm/api/pre-js.js b88499dc303c21fc3f55f2c364a0f814f587b60a95784303881169f9e91c1d5f
 F ext/wasm/api/sqlite3-api-cleanup.js ecdc69dbfccfe26146f04799fcfd4a6f5790d46e7e3b9b6e9b0491f92ed8ae34
 F ext/wasm/api/sqlite3-api-glue.js 056f44b82c126358a0175e08a892d56fadfce177b0d7a0012502a6acf67ea6d5
-F ext/wasm/api/sqlite3-api-oo1.js e9a83489bbb4838ce0aee46eaaa9350e0e25a5b926b565e4f5ae8e840e4fbaed
-F ext/wasm/api/sqlite3-api-opfs.js 38d368e33f470f9ba196f1a2b0c9ce076c930c70df233c345a246f1ad4c26d3b
+F ext/wasm/api/sqlite3-api-oo1.js e4df25e7fd1a0b67a9f3df9eea8cbcbcdecab55be481c903488a9e8dcaf356e4
+F ext/wasm/api/sqlite3-api-opfs.js 69a897eae705816a002f44e8a37693e2ceb7b3e32f9889df2302aeba7df24c70
 F ext/wasm/api/sqlite3-api-prologue.js 08e96d26d329e8c1e08813fe0b84ee93e0e78b087efdd6eb2809ae2672902437
 F ext/wasm/api/sqlite3-api-worker1.js e94ba98e44afccfa482874cd9acb325883ade50ed1f9f9526beb9de1711f182f
 F ext/wasm/api/sqlite3-license-version-header.js a661182fc93fc2cf212dfd0b987f8e138a3ac98f850b1112e29b5fbdaecc87c3
-F ext/wasm/api/sqlite3-opfs-async-proxy.js 1ec10873f1d59d305f6f3b435c50a1b75d693d5fb739b226f3da46fcbb11261a
+F ext/wasm/api/sqlite3-opfs-async-proxy.js 3142ed3a636d9a1a3f4793c6af6373996593c615a3a5fa29de59ba3e0ea45bee
 F ext/wasm/api/sqlite3-wasi.h 25356084cfe0d40458a902afb465df8c21fc4152c1d0a59b563a3fba59a068f9
 F ext/wasm/api/sqlite3-wasm.c 8fc8f47680df0e9a6c0f2f03cb004148645ecc983aa216daba09cb21f7e092a2
 F ext/wasm/api/sqlite3-worker1-promiser.js 0c7a9826dbf82a5ed4e4f7bf7816e825a52aff253afbf3350431f5773faf0e4b
@@ -554,7 +554,7 @@ F ext/wasm/tester1.c-pp.html 74aa9b31c75f12490653f814b53c3dd39f40cd3f70d6a53a716
 F ext/wasm/tester1.c-pp.js 0c129495d057c77788b59715152d51f9bf9002ebbcce759ef8b028272ce3519d
 F ext/wasm/tests/opfs/concurrency/index.html bb9b0f6da86df34c67fa506db9c45b7c4cf0045a211611cc6b8d2b53fa983481
 F ext/wasm/tests/opfs/concurrency/test.js 5993c08657d547d3a26b78ff3480122aed2b7361823bc127e96e558931093aff
-F ext/wasm/tests/opfs/concurrency/worker.js afccb78082b57edb17d5aba0754c823772553395df6f1aed92f82b4d9e3c32de
+F ext/wasm/tests/opfs/concurrency/worker.js cc43ea47bb59582523e3f27c2198247c4adca2fae9a891f84dd3bccfccef2833
 F ext/wasm/version-info.c 3b36468a90faf1bbd59c65fd0eb66522d9f941eedd364fabccd72273503ae7d5
 F ext/wasm/wasmfs.make 8fea9b4f3cde06141de1fc4c586ab405bd32c3f401554f4ebb18c797401a678d
 F install-sh 9d4de14ab9fb0facae2f48780b874848cbf2f895 x
@@ -2059,9 +2059,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 220cc4c6399b772b4ece03305a41b437ef0654d586a8a1c3dc5e7606fd36d655 59a837cfc7f9f96509491c8fc45355d2e8892af25246955e22adec1cbf37327b
-R 6b5395a59674684ae7409c1a67752bd1
-T +closed 59a837cfc7f9f96509491c8fc45355d2e8892af25246955e22adec1cbf37327b
-U dan
-Z 08a51ae8157c87edcee2b45d299f4155
+P 5f135575b923cb59947667071c6af9ff14063c933cedf37d6c2a0a1b86c85032
+R 5ffa2a11b06c4a44d92d27455a2fc3e8
+U stephan
+Z 70f5adf76d87ddf8a488ff2bcd290afb
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index 4c19a5133ea7c52c260a8b4bc3c8eb810cd98287..54a2776a7a9ac1c41cf4cfcab36a86af75fa4054 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-5f135575b923cb59947667071c6af9ff14063c933cedf37d6c2a0a1b86c85032
\ No newline at end of file
+c5b7a9715a13b696ab3ee965aa0a310f59b65f07cecd72faa2e3504bfd8eb632
\ No newline at end of file