OPFS VFS: add the opfs-unlock-asap=1 URI flag which tells the VFS to release implicit locks ASAP instead of during VFS idle time. This improves concurrency notably in the test app but brings a significant performance penalty in speedtest1 (roughly 4x slowdown). This is not the final word in OPFS concurrency, but gets us a step further.

FossilOrigin-Name: 9542f9ce9e023b489e2d93661f719fb0751c1e28f72fded9d3c2156d5777e7b1

1 files changed, 64 insertions, 32 deletions

diff --git a/ext/wasm/api/sqlite3-opfs-async-proxy.js b/ext/wasm/api/sqlite3-opfs-async-proxy.js
index c208932e1..8a3db9c64 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,20 @@ const closeAutoLocks = async ()=>{
 };
 
 /**
+   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 +260,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 +276,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 +424,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 +489,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 +511,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);
@@ -508,10 +524,10 @@ const vfsAsyncImpls = {
     mTimeEnd();
   },
   xOpen: async function(fid/*sqlite3_file pointer*/, filename,
-                        flags/*SQLITE_OPEN_...*/){
+                        flags/*SQLITE_OPEN_...*/,
+                        opfsFlags/*OPFS_...*/){
     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 +542,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 +552,26 @@ 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 =
+        (opfsFlags & state.opfsFlags.OPFS_UNLOCK_ASAP)
+        || state.opfsFlags.defaultUnlockAsap;
+      if(0 /* this block is modelled after something wa-sqlite
+              does but it leads to immediate 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.
+
+           https://www.sqlite.org/uri.html
+        */
+        fh.xLock = "xOpen"/* 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 +588,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 +603,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 +632,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 +670,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 +679,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();
@@ -777,13 +808,13 @@ const waitLoop = async function f(){
      to do other things. If this is too high (e.g. 500ms) then
      even two workers/tabs can easily run into locking errors.
   */
-  const waitTime = 150;
+  const waitTime = 100;
   while(!flagAsyncShutdown){
     try {
       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);
@@ -824,6 +855,7 @@ navigator.storage.getDirectory().then(function(d){
           state.sabS11nView = new Uint8Array(state.sabIO, state.sabS11nOffset, state.sabS11nSize);
           state.opIds = opt.opIds;
           state.sq3Codes = opt.sq3Codes;
+          state.opfsFlags = opt.opfsFlags;
           Object.keys(vfsAsyncImpls).forEach((k)=>{
             if(!Number.isFinite(state.opIds[k])){
               toss("Maintenance required: missing state.opIds[",k,"]");