Initial infrastructure for adding a mode to the OPFS VFS which causes implicit locks to be released ASAP, which increases concurrency at the cost of performance.

FossilOrigin-Name: c5b7a9715a13b696ab3ee965aa0a310f59b65f07cecd72faa2e3504bfd8eb632

1 files changed, 93 insertions, 30 deletions

diff --git a/ext/wasm/api/sqlite3-opfs-async-proxy.js b/ext/wasm/api/sqlite3-opfs-async-proxy.js
index c208932e1..cbe0549b3 100644
--- 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);
@@ -202,6 +202,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
    between locking-related failures and other types, noting that we
@@ -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);