From: stephan <stephan@noemail.net>
Date: Fri, 14 Oct 2022 15:52:29 +0000 (+0000)
Subject: Generic minor cleanups and docs in the OPFS async proxy.
X-Git-Tag: version-3.40.0~160
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e72ddfd89b05c7ed60b82ac019480d1f3a5c7a21;p=thirdparty%2Fsqlite.git

Generic minor cleanups and docs in the OPFS async proxy.

FossilOrigin-Name: a4423ca234453c14eb40db7fe5943f63b30fd9dc2207388e8a2966733a004e9d
---

diff --git a/ext/wasm/sqlite3-opfs-async-proxy.js b/ext/wasm/sqlite3-opfs-async-proxy.js
index d261dd354c..5b29aa3ea5 100644
--- a/ext/wasm/sqlite3-opfs-async-proxy.js
+++ b/ext/wasm/sqlite3-opfs-async-proxy.js
@@ -10,10 +10,10 @@
 
   ***********************************************************************
 
-  An INCOMPLETE and UNDER CONSTRUCTION experiment for OPFS: a Worker
-  which manages asynchronous OPFS handles on behalf of a synchronous
-  API which controls it via a combination of Worker messages,
-  SharedArrayBuffer, and Atomics.
+  An Worker which manages asynchronous OPFS handles on behalf of a
+  synchronous API which controls it via a combination of Worker
+  messages, SharedArrayBuffer, and Atomics. It is the asynchronous
+  counterpart of the API defined in sqlite3-api-opfs.js.
 
   Highly indebted to:
 
@@ -102,9 +102,9 @@ const __openFiles = Object.create(null);
 
 /**
    Expects an OPFS file path. It gets resolved, such that ".."
-   components are properly expanded, and returned. If the 2nd
-   are is true, it's returned as an array of path elements,
-   else it's returned as an absolute path string.
+   components are properly expanded, and returned. If the 2nd arg is
+   true, the result is returned as an array of path elements, else an
+   absolute path string is returned.
 */
 const getResolvedPath = function(filename,splitIt){
   const p = new URL(
@@ -115,9 +115,9 @@ const getResolvedPath = function(filename,splitIt){
 
 /**
    Takes the absolute path to a filesystem element. Returns an array
-   of [handleOfContainingDir, filename]. If the 2nd argument is
-   truthy then each directory element leading to the file is created
-   along the way. Throws if any creation or resolution fails.
+   of [handleOfContainingDir, filename]. If the 2nd argument is truthy
+   then each directory element leading to the file is created along
+   the way. Throws if any creation or resolution fails.
 */
 const getDirForFilename = async function f(absFilename, createDirs = false){
   const path = getResolvedPath(absFilename, true);
@@ -171,6 +171,16 @@ const getSyncHandle = async (fh)=>{
   return fh.syncHandle;
 };
 
+/**
+   If the given file-holding object has a sync handle attached to it,
+   that handle is remove and asynchronously closed. Though it may
+   sound sensible to continue work as soon as the close() returns
+   (noting that it's asynchronous), doing so can cause operations
+   performed soon afterwards, e.g. a call to getSyncHandle() to fail
+   because they may happen out of order from the close(). OPFS does
+   not guaranty that the actual order of operations is retained in
+   such cases. i.e.  always "await" on the result of this function.
+*/
 const closeSyncHandle = async (fh)=>{
   if(fh.syncHandle){
     log("Closing sync handle for",fh.filenameAbs);
@@ -197,43 +207,50 @@ const affirmNotRO = function(opName,fh){
   if(fh.readOnly) toss(opName+"(): File is read-only: "+fh.filenameAbs);
 };
 
-
-const opTimer = Object.create(null);
-opTimer.op = undefined;
-opTimer.start = undefined;
+/**
+   We track 2 different timers: the "metrics" timer records how much
+   time we spend performing work. The "wait" timer records how much
+   time we spend waiting on the underlying OPFS timer. See the calls
+   to mTimeStart(), mTimeEnd(), wTimeStart(), and wTimeEnd()
+   throughout this file to see how they're used.
+*/
+const __mTimer = Object.create(null);
+__mTimer.op = undefined;
+__mTimer.start = undefined;
 const mTimeStart = (op)=>{
-  opTimer.start = performance.now();
-  opTimer.op = op;
+  __mTimer.start = performance.now();
+  __mTimer.op = op;
   //metrics[op] || toss("Maintenance required: missing metrics for",op);
   ++metrics[op].count;
 };
 const mTimeEnd = ()=>(
-  metrics[opTimer.op].time += performance.now() - opTimer.start
+  metrics[__mTimer.op].time += performance.now() - __mTimer.start
 );
-const waitTimer = Object.create(null);
-waitTimer.op = undefined;
-waitTimer.start = undefined;
+const __wTimer = Object.create(null);
+__wTimer.op = undefined;
+__wTimer.start = undefined;
 const wTimeStart = (op)=>{
-  waitTimer.start = performance.now();
-  waitTimer.op = op;
+  __wTimer.start = performance.now();
+  __wTimer.op = op;
   //metrics[op] || toss("Maintenance required: missing metrics for",op);
 };
 const wTimeEnd = ()=>(
-  metrics[waitTimer.op].wait += performance.now() - waitTimer.start
+  metrics[__wTimer.op].wait += performance.now() - __wTimer.start
 );
 
 /**
-   Set to true by the 'opfs-async-shutdown' command to quite the wait loop.
-   This is only intended for debugging purposes: we cannot inspect this
-   file's state while the tight waitLoop() is running.
+   Gets set to true by the 'opfs-async-shutdown' command to quit the
+   wait loop. This is only intended for debugging purposes: we cannot
+   inspect this file's state while the tight waitLoop() is running and
+   need a way to stop that loop for introspection purposes.
 */
 let flagAsyncShutdown = false;
 
 
 /**
    Asynchronous wrappers for sqlite3_vfs and sqlite3_io_methods
-   methods. Maintenance reminder: members are in alphabetical order
-   to simplify finding them.
+   methods, as well as helpers like mkdir(). Maintenance reminder:
+   members are in alphabetical order to simplify finding them.
 */
 const vfsAsyncImpls = {
   'opfs-async-metrics': async ()=>{
@@ -369,11 +386,13 @@ const vfsAsyncImpls = {
     const fh = __openFiles[fid];
     let rc = 0;
     if( !fh.syncHandle ){
+      wTimeStart('xLock');
       try { await getSyncHandle(fh) }
       catch(e){
         state.s11n.storeException(1,e);
         rc = state.sq3Codes.SQLITE_IOERR;
       }
+      wTimeEnd();
     }
     storeAndNotify('xLock',rc);
     mTimeEnd();
@@ -423,11 +442,11 @@ const vfsAsyncImpls = {
   },
   xRead: async function(fid,n,offset){
     mTimeStart('xRead');
-    let rc = 0;
+    let rc = 0, nRead;
     const fh = __openFiles[fid];
     try{
       wTimeStart('xRead');
-      const nRead = (await getSyncHandle(fh)).read(
+      nRead = (await getSyncHandle(fh)).read(
         fh.sabView.subarray(0, n),
         {at: Number(offset)}
       );
@@ -437,6 +456,7 @@ const vfsAsyncImpls = {
         rc = state.sq3Codes.SQLITE_IOERR_SHORT_READ;
       }
     }catch(e){
+      if(undefined===nRead) wTimeEnd();
       error("xRead() failed",e,fh);
       state.s11n.storeException(1,e);
       rc = state.sq3Codes.SQLITE_IOERR_READ;
@@ -454,9 +474,8 @@ const vfsAsyncImpls = {
         await fh.syncHandle.flush();
       }catch(e){
         state.s11n.storeException(2,e);
-      }finally{
-        wTimeEnd();
       }
+      wTimeEnd();
     }
     storeAndNotify('xSync',rc);
     mTimeEnd();
@@ -484,13 +503,13 @@ const vfsAsyncImpls = {
     const fh = __openFiles[fid];
     if( state.sq3Codes.SQLITE_LOCK_NONE===lockType
         && fh.syncHandle ){
+      wTimeStart('xUnlock');
       try { await closeSyncHandle(fh) }
       catch(e){
         state.s11n.storeException(1,e);
         rc = state.sq3Codes.SQLITE_IOERR;
-        /* Maybe we want to not report this? "Destructors do not
-           throw." */
       }
+      wTimeEnd();
     }
     storeAndNotify('xUnlock',rc);
     mTimeEnd();
@@ -511,9 +530,8 @@ const vfsAsyncImpls = {
       error("xWrite():",e,fh);
       state.s11n.storeException(1,e);
       rc = state.sq3Codes.SQLITE_IOERR_WRITE;
-    }finally{
-      wTimeEnd();
     }
+    wTimeEnd();
     storeAndNotify('xWrite',rc);
     mTimeEnd();
   }
@@ -632,7 +650,7 @@ const waitLoop = async function f(){
     const o = Object.create(null);
     opHandlers[state.opIds[k]] = o;
     o.key = k;
-    o.f = vi || toss("No vfsAsyncImpls[",k,"]");
+    o.f = vi;
   }
   /**
      waitTime is how long (ms) to wait for each Atomics.wait().
@@ -640,8 +658,6 @@ const waitLoop = async function f(){
      to do other things.
   */
   const waitTime = 1000;
-  let lastOpTime = performance.now();
-  let now;
   while(!flagAsyncShutdown){
     try {
       if('timed-out'===Atomics.wait(
@@ -649,7 +665,6 @@ const waitLoop = async function f(){
       )){
         continue;
       }
-      lastOpTime = performance.now();
       const opId = Atomics.load(state.sabOPView, state.opIds.whichOp);
       Atomics.store(state.sabOPView, state.opIds.whichOp, 0);
       const hnd = opHandlers[opId] ?? toss("No waitLoop handler for whichOp #",opId);
@@ -663,7 +678,7 @@ const waitLoop = async function f(){
     }catch(e){
       error('in waitLoop():',e);
     }
-  };
+  }
 };
 
 navigator.storage.getDirectory().then(function(d){
diff --git a/manifest b/manifest
index 5e6a5ac9b8..259868c45d 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Fix\sa\sproblem\sin\sthe\sLIKE\sand\sGLOB\soperators\sthat\smay\soccur\swhen\sthe\scharacter\simmediately\sfollowing\sa\s"%"\sor\s"*"\swildcard\sis\sU+80.\sReported\sby\s[forum:61bf7ccbdf].
-D 2022-10-14T15:10:36.387
+C Generic\sminor\scleanups\sand\sdocs\sin\sthe\sOPFS\sasync\sproxy.
+D 2022-10-14T15:52:29.231
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -519,7 +519,7 @@ F ext/wasm/speedtest1.html e4cb5d722b494104fc1249e7c008ca018f820a784833c51004c95
 F ext/wasm/split-speedtest1-script.sh a3e271938d4d14ee49105eb05567c6a69ba4c1f1293583ad5af0cd3a3779e205 x
 F ext/wasm/sql/000-mandelbrot.sql 775337a4b80938ac8146aedf88808282f04d02d983d82675bd63d9c2d97a15f0
 F ext/wasm/sql/001-sudoku.sql 35b7cb7239ba5d5f193bc05ec379bcf66891bce6f2a5b3879f2f78d0917299b5
-F ext/wasm/sqlite3-opfs-async-proxy.js e552d9290509644929cd7bfde3381a0eac85d17b4d82dbb11e86fae5336215b6
+F ext/wasm/sqlite3-opfs-async-proxy.js 206ce6bbc3c30ad51a37d9c25e3a2712e70b586e0f9a2cf8cb0b9619017c2671
 F ext/wasm/sqlite3-worker1-promiser.js 307d7837420ca6a9d3780dfc81194f1c0715637e6d9540e935514086b96913d8
 F ext/wasm/sqlite3-worker1.js 466e9bd39409ab03f3e00999887aaffc11e95b416e2689596e3d7f1516673fdf
 F ext/wasm/test-opfs-vfs.html eb69dda21eb414b8f5e3f7c1cc0f774103cc9c0f87b2d28a33419e778abfbab5
@@ -2031,8 +2031,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 32fd4ac382f774189ac34f6fff80e55e6e56dd2aa67b0db88d5a88324f17f6ff
-R ad7e356fc9072ccedd7e8855b564b667
-U dan
-Z 16dbeccb3b1fdf0272ff90de38793516
+P 2da677c45b643482eec39e4db7079c772760bc966dc71bf6c01658cc468f5823
+R bd5efaaf882b3ae0c03e0e5a6693b37e
+U stephan
+Z c8a3d40e488aded9faa8a7e4d2f0c359
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index 8828e08bae..de485659dc 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-2da677c45b643482eec39e4db7079c772760bc966dc71bf6c01658cc468f5823
\ No newline at end of file
+a4423ca234453c14eb40db7fe5943f63b30fd9dc2207388e8a2966733a004e9d
\ No newline at end of file