From aafa022f5b5371d83ee3478c17e0c7f2faf1ce62 Mon Sep 17 00:00:00 2001 From: stephan Date: Thu, 10 Nov 2022 11:35:10 +0000 Subject: OPFS: if an op which needs a lock is called when no lock has been obtained, automatically lock it at the start of the op and unlock it at the end of that op. This is an attempt to alleviate the cross-tab contention described in [forum post 58a377083cd24a|forum:58a377083cd24a] but it increases speedtest1 run time by approximately 4x. Perhaps auto-lock can be combined with the older idle-time-based auto-unlock to unlock such locks (but not those from xLock()) to improve this? FossilOrigin-Name: 46304ba057707c3b072b6e7bb8c4af774f653aa5814099f0866cd87b2b73abeb --- ext/wasm/api/sqlite3-opfs-async-proxy.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'ext/wasm/api/sqlite3-opfs-async-proxy.js') diff --git a/ext/wasm/api/sqlite3-opfs-async-proxy.js b/ext/wasm/api/sqlite3-opfs-async-proxy.js index 09c56ff1d..9c0bf4cbe 100644 --- a/ext/wasm/api/sqlite3-opfs-async-proxy.js +++ b/ext/wasm/api/sqlite3-opfs-async-proxy.js @@ -214,6 +214,24 @@ const closeSyncHandle = async (fh)=>{ } }; +/** + A proxy for closeSyncHandle() which is guaranteed to not throw. + + This function is part of a lock/unlock step in functions which + require a sync access handle but may be called without xLock() + having been called first. Such calls need to release that + handle to avoid locking the file for all of time. This is an + _attempt_ at reducing cross-tab contention but it may prove + to be more of a problem than a solution and may need to be + removed. +*/ +const closeSyncHandleNoThrow = async (fh)=>{ + try{await closeSyncHandle(fh)} + catch(e){ + warn("closeSyncHandleNoThrow() ignoring:",e,fh); + } +}; + /** Stores the given value at state.sabOPView[state.opIds.rc] and then Atomics.notify()'s it. @@ -403,6 +421,7 @@ const vfsAsyncImpls = { mTimeStart('xFileSize'); const fh = __openFiles[fid]; let rc; + const hadLock = !!fh.syncHandle; wTimeStart('xFileSize'); try{ affirmLocked('xFileSize',fh); @@ -413,6 +432,7 @@ const vfsAsyncImpls = { state.s11n.storeException(2,e); rc = state.sq3Codes.SQLITE_IOERR; } + if(!hadLock) closeSyncHandleNoThrow(fh); wTimeEnd(); storeAndNotify('xFileSize', rc); mTimeEnd(); @@ -483,6 +503,7 @@ const vfsAsyncImpls = { mTimeStart('xRead'); let rc = 0, nRead; const fh = __openFiles[fid]; + const hadLock = !!fh.syncHandle; try{ affirmLocked('xRead',fh); wTimeStart('xRead'); @@ -501,6 +522,7 @@ const vfsAsyncImpls = { state.s11n.storeException(1,e); rc = state.sq3Codes.SQLITE_IOERR_READ; } + if(!hadLock) closeSyncHandleNoThrow(fh); storeAndNotify('xRead',rc); mTimeEnd(); }, @@ -525,6 +547,7 @@ const vfsAsyncImpls = { mTimeStart('xTruncate'); let rc = 0; const fh = __openFiles[fid]; + const hadLock = !!fh.syncHandle; wTimeStart('xTruncate'); try{ affirmLocked('xTruncate',fh); @@ -535,6 +558,7 @@ const vfsAsyncImpls = { state.s11n.storeException(2,e); rc = state.sq3Codes.SQLITE_IOERR_TRUNCATE; } + if(!hadLock) closeSyncHandleNoThrow(fh); wTimeEnd(); storeAndNotify('xTruncate',rc); mTimeEnd(); @@ -561,6 +585,7 @@ const vfsAsyncImpls = { mTimeStart('xWrite'); let rc; const fh = __openFiles[fid]; + const hadLock = !!fh.syncHandle; wTimeStart('xWrite'); try{ affirmLocked('xWrite',fh); @@ -575,6 +600,7 @@ const vfsAsyncImpls = { state.s11n.storeException(1,e); rc = state.sq3Codes.SQLITE_IOERR_WRITE; } + if(!hadLock) closeSyncHandleNoThrow(fh); wTimeEnd(); storeAndNotify('xWrite',rc); mTimeEnd(); -- cgit v1.2.3 From da2641597a401b2a6c19dc6ee860f9f25ec1581b Mon Sep 17 00:00:00 2001 From: stephan Date: Thu, 10 Nov 2022 13:14:30 +0000 Subject: Rework automatically acquired OPFS locks to be released during idle time. This eliminates the performance hit reported in [46304ba057707c]. FossilOrigin-Name: a7fe91afca473fe55c983bc81d214df4ef3699863c7423fa4b6b9cde23d6a3b4 --- ext/wasm/api/sqlite3-opfs-async-proxy.js | 74 ++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 22 deletions(-) (limited to 'ext/wasm/api/sqlite3-opfs-async-proxy.js') diff --git a/ext/wasm/api/sqlite3-opfs-async-proxy.js b/ext/wasm/api/sqlite3-opfs-async-proxy.js index 9c0bf4cbe..e4657484e 100644 --- a/ext/wasm/api/sqlite3-opfs-async-proxy.js +++ b/ext/wasm/api/sqlite3-opfs-async-proxy.js @@ -44,6 +44,7 @@ if(self.window === self){ this API. */ const state = Object.create(null); + /** verbose: @@ -96,13 +97,27 @@ metrics.dump = ()=>{ }; /** - Map of sqlite3_file pointers (integers) to metadata related to a - given OPFS file handles. The pointers are, in this side of the - interface, opaque file handle IDs provided by the synchronous - part of this constellation. Each value is an object with a structure - demonstrated in the xOpen() impl. + __openFiles is a map of sqlite3_file pointers (integers) to + metadata related to a given OPFS file handles. The pointers are, in + this side of the interface, opaque file handle IDs provided by the + synchronous part of this constellation. Each value is an object + with a structure demonstrated in the xOpen() impl. */ const __openFiles = Object.create(null); +/** + __autoLocks 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 + handle obtained via xLock(), or subsequently xLock()'d after + auto-acquisition, will not be released until xUnlock() is called. + + Maintenance reminder: if we relinquish auto-locks at the end of the + operation which acquires them, we pay a massive performance + 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(); /** Expects an OPFS file path. It gets resolved, such that ".." @@ -191,6 +206,10 @@ const getSyncHandle = async (fh)=>{ } } log("Got sync handle for",fh.filenameAbs,'in',performance.now() - t,'ms'); + if(!fh.xLock){ + __autoLocks.add(fh.fid); + log("Auto-locked",fh.fid,fh.filenameAbs); + } } return fh.syncHandle; }; @@ -210,6 +229,8 @@ const closeSyncHandle = async (fh)=>{ log("Closing sync handle for",fh.filenameAbs); const h = fh.syncHandle; delete fh.syncHandle; + delete fh.xLock; + __autoLocks.delete(fh.fid); return h.close(); } }; @@ -360,9 +381,10 @@ const vfsAsyncImpls = { xClose: async function(fid/*sqlite3_file pointer*/){ const opName = 'xClose'; mTimeStart(opName); + __autoLocks.delete(fid); const fh = __openFiles[fid]; let rc = 0; - wTimeStart('xClose'); + wTimeStart(opName); if(fh){ delete __openFiles[fid]; await closeSyncHandle(fh); @@ -421,7 +443,6 @@ const vfsAsyncImpls = { mTimeStart('xFileSize'); const fh = __openFiles[fid]; let rc; - const hadLock = !!fh.syncHandle; wTimeStart('xFileSize'); try{ affirmLocked('xFileSize',fh); @@ -432,7 +453,6 @@ const vfsAsyncImpls = { state.s11n.storeException(2,e); rc = state.sq3Codes.SQLITE_IOERR; } - if(!hadLock) closeSyncHandleNoThrow(fh); wTimeEnd(); storeAndNotify('xFileSize', rc); mTimeEnd(); @@ -442,12 +462,17 @@ const vfsAsyncImpls = { mTimeStart('xLock'); const fh = __openFiles[fid]; let rc = 0; + const oldLockType = fh.xLock; + fh.xLock = lockType; if( !fh.syncHandle ){ wTimeStart('xLock'); - try { await getSyncHandle(fh) } - catch(e){ + try { + await getSyncHandle(fh); + __autoLocks.delete(fid); + }catch(e){ state.s11n.storeException(1,e); rc = state.sq3Codes.SQLITE_IOERR_LOCK; + fh.xLock = oldLockType; } wTimeEnd(); } @@ -481,6 +506,7 @@ const vfsAsyncImpls = { */ wTimeEnd(); __openFiles[fid] = Object.assign(Object.create(null),{ + fid: fid, filenameAbs: filename, filenamePart: filenamePart, dirHandle: hDir, @@ -503,7 +529,6 @@ const vfsAsyncImpls = { mTimeStart('xRead'); let rc = 0, nRead; const fh = __openFiles[fid]; - const hadLock = !!fh.syncHandle; try{ affirmLocked('xRead',fh); wTimeStart('xRead'); @@ -522,7 +547,6 @@ const vfsAsyncImpls = { state.s11n.storeException(1,e); rc = state.sq3Codes.SQLITE_IOERR_READ; } - if(!hadLock) closeSyncHandleNoThrow(fh); storeAndNotify('xRead',rc); mTimeEnd(); }, @@ -547,7 +571,6 @@ const vfsAsyncImpls = { mTimeStart('xTruncate'); let rc = 0; const fh = __openFiles[fid]; - const hadLock = !!fh.syncHandle; wTimeStart('xTruncate'); try{ affirmLocked('xTruncate',fh); @@ -558,7 +581,6 @@ const vfsAsyncImpls = { state.s11n.storeException(2,e); rc = state.sq3Codes.SQLITE_IOERR_TRUNCATE; } - if(!hadLock) closeSyncHandleNoThrow(fh); wTimeEnd(); storeAndNotify('xTruncate',rc); mTimeEnd(); @@ -585,7 +607,6 @@ const vfsAsyncImpls = { mTimeStart('xWrite'); let rc; const fh = __openFiles[fid]; - const hadLock = !!fh.syncHandle; wTimeStart('xWrite'); try{ affirmLocked('xWrite',fh); @@ -600,7 +621,6 @@ const vfsAsyncImpls = { state.s11n.storeException(1,e); rc = state.sq3Codes.SQLITE_IOERR_WRITE; } - if(!hadLock) closeSyncHandleNoThrow(fh); wTimeEnd(); storeAndNotify('xWrite',rc); mTimeEnd(); @@ -636,7 +656,7 @@ const initS11n = ()=>{ default: toss("Invalid type ID:",tid); } }; - state.s11n.deserialize = function(){ + state.s11n.deserialize = function(clear=false){ ++metrics.s11n.deserialize.count; const t = performance.now(); const argc = viewU8[0]; @@ -661,6 +681,7 @@ const initS11n = ()=>{ rc.push(v); } } + if(clear) viewU8[0] = 0; //log("deserialize:",argc, rc); metrics.s11n.deserialize.time += performance.now() - t; return rc; @@ -727,21 +748,30 @@ const waitLoop = async function f(){ We need to wake up periodically to give the thread a chance to do other things. */ - const waitTime = 1000; + const waitTime = 500; while(!flagAsyncShutdown){ try { if('timed-out'===Atomics.wait( state.sabOPView, state.opIds.whichOp, 0, waitTime )){ + if(__autoLocks.size){ + /* Release all auto-locks. */ + for(const fid of __autoLocks){ + const fh = __openFiles[fid]; + await closeSyncHandleNoThrow(fh); + log("Auto-unlocked",fid,fh.filenameAbs); + } + } continue; } 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); - const args = state.s11n.deserialize() || []; - state.s11n.serialize(/* clear s11n to keep the caller from - confusing this with an exception string - written by the upcoming operation */); + const args = state.s11n.deserialize( + true /* clear s11n to keep the caller from confusing this with + an exception string written by the upcoming + operation */ + ) || []; //warn("waitLoop() whichOp =",opId, hnd, args); if(hnd.f) await hnd.f(...args); else error("Missing callback for opId",opId); -- cgit v1.2.3