diff --git a/ext/wasm/api/sqlite3-api-oo1.js b/ext/wasm/api/sqlite3-api-oo1.js index 02ce9c0ced..bc32ec1067 100644 --- 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 deb4c923ab..53f68a1d58 100644 --- 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 c208932e17..cbe0549b3e 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); @@ -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 19b0a068e7..9b4898f4c3 100644 --- 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 3ecb281109..d6e43cfa43 100644 --- 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 4c19a5133e..54a2776a7a 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -5f135575b923cb59947667071c6af9ff14063c933cedf37d6c2a0a1b86c85032 \ No newline at end of file +c5b7a9715a13b696ab3ee965aa0a310f59b65f07cecd72faa2e3504bfd8eb632 \ No newline at end of file