From: stephan Date: Sun, 23 Nov 2025 15:38:34 +0000 (+0000) Subject: Initial work on eliminating the superfluous-for-transient-storage kvvfs key prefixes... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d855948547147dc5f837a460c497090ba1d70bbb;p=thirdparty%2Fsqlite.git Initial work on eliminating the superfluous-for-transient-storage kvvfs key prefixes and implementing kvvfs db import. FossilOrigin-Name: 6bc64059410b1868b7a3576e16d03c02e7c007a2be8b313e386eeb2e2a35c258 --- diff --git a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js index 5365cbc2c2..8c454ffc0e 100644 --- a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js +++ b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js @@ -55,8 +55,9 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ /** Implementation of JS's Storage interface for use as backing store - of the kvvfs. Storage's constructor cannot be legally called from - JS, making it impossible to directly subclass Storage. + of the kvvfs. Storage is a native class and its constructor + cannot be legally called from JS, making it impossible to + directly subclass Storage. This impl simply proxies a plain, prototype-less Object, suitable for JSON-ing. @@ -114,39 +115,55 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ /* Start off with mappings for well-known names. */ localThread: { refc: 3/*never reaches 0*/, - s: new TransientStorage + s: new TransientStorage, + files: [/*KVVfsFile instances currently using this storage*/] } }); - if( globalThis.localStorage ){ - cache.jzClassToStorage.local = - { - refc: 3/*never reaches 0*/, - s: globalThis.localStorage, - /* If useFullZClass is true, kvvfs storage keys are in - the form kvvfs-{zClass}-*, else they lack the "-{zClass}" - part. local/session storage must use the long form for - backwards compatibility. */ - useFullZClass: true - }; + if( globalThis.localStorage instanceof globalThis.Storage ){ + cache.jzClassToStorage.local = { + refc: 3/*never reaches 0*/, + s: globalThis.localStorage, + /* This is the storage prefix used for kvvfs keys. It is + "kvvfs-STORAGENAME-" for local/session storage and an empty + string for transient storage. local/session storage must + use the long form for backwards compatibility. + + This prefix mirrors the one generated by os_kv.c's + kvrecordMakeKey() and must stay in sync with that one. + */ + keyPrefix: "kvvfs-local-", + files: [] + }; } - if( globalThis.sessionStorage ){ - cache.jzClassToStorage.session = - { - refc: 3/*never reaches 0*/, - s: globalThis.sessionStorage, - useFullZClass: true - } + if( globalThis.sessionStorage instanceof globalThis.Storage ){ + cache.jzClassToStorage.session = { + refc: 3/*never reaches 0*/, + s: globalThis.sessionStorage, + keyPrefix: "kvvfs-session-", + files: [] + } } for(const k of Object.keys(cache.jzClassToStorage)){ /* Journals in kvvfs are are stored as individual records within - their Storage-ish object, named "kvvfs-${zClass}-jrnl". We - always create mappings for both the db file's name and the - journal's name referring to the same Storage object. */ + their Storage-ish object, named "KEYPREFIXjrnl" (see above + re. KEYPREFIX). We always map the db and its journal to the + same Storage object. */ const orig = cache.jzClassToStorage[k]; orig.jzClass = k; cache.jzClassToStorage[k+'-journal'] = orig; } + const kvvfsIsPersistentName = (v)=>'local'===v || 'session'===v; + /** + Keys in kvvfs have a prefix of "kvvfs-NAME", where NAME is the db + name. This key is redundant in JS but it's how kvvfs works (it + saves each key to a separate file, so needs a + distinct-per-db-name namespace). We retain this prefix in 'local' + and 'session' storage for backwards compatibility but elide them + from "v2" transient storage, where they're superfluous. + */ + const kvvfsKeyPrefix = (v)=>kvvfsIsPersistentName(v) ? 'kvvfs-'+v+'-' : ''; + /** Internal helper for sqlite3_js_kvvfs_clear() and friends. Its argument should be one of ('local','session',"") or the name of @@ -155,30 +172,34 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ It returns an object in the form: .prefix = the key prefix for this storage. Typically - ("kvvfs-"+which) for persistent storage and "kvvfs-" for - transient. (The former is historical, retained for backwards - compatibility.) + ("kvvfs-"+which) for local/sessionStorage and "" for transient + storage. (The former is historical, retained for backwards + compatibility.) If which is falsy then the prefix is "kvvfs-" for + backwards compatibility (it will match keys for both local- and + sessionStorage, but not transient storage). .stores = [ array of Storage-like objects ]. Will only have >1 element if which is falsy, in which case it contains (if called - from the main thread) localStorage and sessionStorage. It will - be empty if no mapping is found. + from the main thread) localStorage and sessionStorage. It will be + empty if no mapping is found or those objects are not available + in the current environment (e.g. a worker thread). */ const kvvfsWhich = function callee(which){ const rc = Object.assign(Object.create(null),{ - prefix: 'kvvfs-' + which, stores: [] }); if( which ){ const s = cache.jzClassToStorage[which]; if( s ){ //debug("kvvfsWhich",s.jzClass,rc.prefix, s.s); - if( !s.useFullZClass ){ - rc.prefix = 'kvvfs-'; - } + rc.prefix = s.keyPrefix ?? ''; rc.stores.push(s.s); + }else{ + rc.prefix = undefined; } }else{ + // Legacy behavior: return both local and session storage. + rc.prefix = 'kvvfs-'; if( globalThis.sessionStorage ) rc.stores.push(globalThis.sessionStorage); if( globalThis.localStorage ) rc.stores.push(globalThis.localStorage); } @@ -207,6 +228,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ capi.sqlite3_js_kvvfs_clear = function(which=""){ let rc = 0; const store = kvvfsWhich(which); + const keyPrefix = store.prefix; store.stores.forEach((s)=>{ const toRm = [] /* keys to remove */; let i, n = s.length; @@ -214,7 +236,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ for( i = 0; i < n; ++i ){ const k = s.key(i); //debug("kvvfs_clear ?",k); - if(k.startsWith(store.prefix)) toRm.push(k); + if(!keyPrefix || k.startsWith(keyPrefix)) toRm.push(k); } toRm.forEach((kk)=>s.removeItem(kk)); rc += toRm.length; @@ -266,20 +288,28 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ const keyForStorage = (store, zClass, zKey)=>{ //debug("keyForStorage(",store, wasm.cstrToJs(zClass), wasm.cstrToJs(zKey)); return wasm.exports.sqlite3__wasm_kvvfsMakeKeyOnPstack( - store.useFullZClass ? zClass : null, zKey + store.keyPrefix ? zClass : null, zKey ); }; + /* We use this for the many small key allocations we need. + TODO: prealloc a buffer on demand for this. We know its + max size from kvvfsMethods.$nKeySize. */ const pstack = wasm.pstack; + /** + Returns the storage object mapped to the given C-string + zClass. + */ const storageForZClass = (zClass)=>cache.jzClassToStorage[wasm.cstrToJs(zClass)]; - const pFileHandles = new Map( - /* sqlite3_file pointers => objects, each of which has: - .s = Storage object - .f = KVVfsFile instance - .n = JS-string form of f.$zClass - */ - ); + /** + sqlite3_file pointers => objects, each of which has: + + .s = Storage object + .f = KVVfsFile instance + .n = JS-string form of f.$zClass + */ + const pFileHandles = new Map(); if( sqlite3.__isUnderTest ){ sqlite3.kvvfsStuff = { @@ -459,6 +489,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ //debug("xOpen", jzClass, s); if( s ){ ++s.refc; + s.files.push(f); }else{ /* TODO: a url flag which tells it to keep the storage around forever so that future xOpen()s get the same @@ -476,7 +507,8 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ will follow soon enough and bump the refcount. If we start at 2 here, that pending open will increment it again. */, - s: new TransientStorage + s: new TransientStorage, + files: [f] }); debug("xOpen installed storage handles [", jzClass, other,"]", s); @@ -535,7 +567,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ If we poke 0 then no problem... except that xAccess() doesn't report the truth. Same effect if we move that to the native impl - os_kv.c:kvvfsAccess(). */ + os_kv.c's kvvfsAccess(). */ ); } debug("xAccess", jzName, drc, pResOut, wasm.peek32(pResOut)); @@ -598,6 +630,8 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ pFileHandles.delete(pFile); const s = cache.jzClassToStorage[h.n]; util.assert(s, "Missing jzClassToStorage["+h.n+"]"); + util.assert(h.f, "Missing KVVfsFile handle for "+h.n); + s.files = s.files.filter((v)=>v!==h.f); if( 0===--s.refc ){ const other = h.f.$isJournal ? h.n.replace(cache.rxJournalSuffix,'') @@ -605,8 +639,10 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ debug("cleaning up storage handles [", h.n, other,"]",s); delete cache.jzClassToStorage[h.n]; delete cache.jzClassToStorage[other]; - delete s.s; - delete s.refc; + if( !sqlite3.__isUnderTest ){ + delete s.s; + delete s.refc; + } } originalIoMethods(h.f).xClose(pFile); h.f.dispose(); @@ -768,60 +804,123 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ - "name": the name of the db. This is 'local' or 'session' for localStorage resp. sessionStorage, and an arbitrary name for - transient storage. + transient storage. This propery may be changed before passing + this object to importFromObject() in order to import into a + different storage object. - "timestamp": the time this function was called, in Unix - epoch milliseconds. - - - "data": An object holding the raw encoded state. It has the - following properties: + epoch milliseconds. - - "kvvfs[-X]-sz" = the decoded size of the db. Its encoded - size may vary wildly from that in either direction, - depending largely on the ratio of empty space to data. + - "size": the unencoded db size. - - "kvvfs[-X]-jrnl" = if includeJournal is true and the db has - a journal, it is stored in this record. If is has no - journal, or includeJournal is false, this key is not set. + - "journal": if includeJournal is true and this db has a + journal, it is stored as a string here, otherwise this property + is not set. - - "kvvfs[-X]-###" = one encoded page of the db, with ### - corresponding to the page number. + - "pages": An array holddig the object holding the raw encoded + db pages in their proper order. - The [-X] parts are only set for localStorage and sessionStorage - back-ends and the X of each is 'local' or 'session'. That is: - the keys contain the storage back-end's name because of how the - underlying native VFS works (each key goes in its own file so - it must be distinct per storage name). That part is retained - here for backwards compatibility - transient storage objects - elide that part. + Throws if this db is not opened. The encoding of the underlying database is not part of this - interface - it is simply passed on as-is. + interface - it is simply passed on as-is. Interested parties + are directed to src/os_kv.c in the SQLite source tree. - Throws if this db is not opened. + Trivia: for non-trivial databases, this object's JSON encoding + will be slightly smaller that the full db, as this + representation strips out some repetitive parts. Added in version 3.?? (tenatively 3.52). */ - jdb.prototype.exportToObject = function(includeJournal=false){ + jdb.prototype.exportToObject = function(includeJournal=true){ this.affirmOpen(); const store = cache.jzClassToStorage[this.affirmOpen().filename]; - const rx = includeJournal ? undefined : /^kvvfs(-(local|session))?-jrnl$/; - if( store ){ - const s = store.s; - const rc = Object.assign(Object.create(null),{ - name: this.filename, - timestamp: (new Date()).valueOf(), - data:Object.create(null) - }); - let i = 0, n = s.length; - for( ; i < n; ++i ){ - const k = s.key(i); - if( !rx || !rx.test(k) ){ - rc.data[k] = s.getItem(k); + const rxTail = /^kvvfs(-(local|session))?-(\w+)/; + if( !store ){ + util.toss3(capi.SQLITE_ERROR,"kvvfs db '", + this.filename,"' has no storage object."); + } + const s = store.s; + const rc = Object.assign(Object.create(null),{ + name: this.filename, + timestamp: (new Date()).valueOf(), + pages: [] + }); + const pages = Object.create(null); + const keyPrefix = kvvfsKeyPrefix(rc.name); + let i = 0, n = s.length; + for( ; i < n; ++i ){ + const k = s.key(i); + if( !keyPrefix || k.startsWith(keyPrefix) ){ + const m = rxTail.exec(k); + let kk = m[3]; + switch( kk ){ + case 'jrnl': + if( includeJournal ) rc.journal = s.getItem(k); + break; + case 'sz': + rc.size = +s.getItem(k); + break; + default: + kk = +kk /* coerce to number */; + if( !util.isInt32(kk) || kk<=0 ){ + util.toss3(capi.SQLITE_RANGE, "Malformed kvvfs key: "+k); + } + pages[kk] = s.getItem(k); + break; } } - return rc; } + /* Now sort the page numbers and move them into an array. In JS + property keys are always strings, so we have to coerce them to + numbers so we can get them sorted properly for the array. */ + Object.keys(pages).map((v)=>+v).sort().forEach( + (v)=>rc.pages.push(pages[v]) + ); + return rc; + }; + + /** + The counterpart of exportToObject(). Its argument must be + the result of exportToObject(). + + This necessarily wipes out the whole database storage, so + invoking this while the db is in active use invokes undefined + behavior. + + Throws on error. Returns this object on success. + + FIXMEs: + + - We need the page size in the export so that we can reset it, + if needed, on the import. + + - We need to ensure that the native-size KVVfsFile::szDb and + KVVfsFile::szPage get set to -1 for all open instances so that + they re-read the db size. + */ + jdb.prototype.importFromObject = function(exp){ + this.affirmOpen(); + if( !exp?.timestamp + || !exp.name + || undefined===exp.size + || !Array.isArray(exp.pages) ){ + util.toss3(capi.SQLITE_MISUSE, "Malformed export object."); + } + warn("importFromObject() is incomplete"); + this.clearStorage(); + const store = kvvfsWhich(this.filename); + util.assert(store?.s, "Somehow missing a storage object for",this.filename); + const keyPrefix = kvvfsKeyPrefix(this.filename); + try{ + s.setItem(keyPrefix+'sz', exp.size); + if( exp.journal ) s.setItem(keyPrefix+'jrnl', exp.journal); + exp.pages.forEach((v,ndx)=>s.setItem(keyPrefix+(ndx+1))); + }catch(e){ + this.clearStorage(); + throw e; + } + return this; }; if( sqlite3.__isUnderTest ){ diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index 0092548ea2..99ec432c7b 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -2945,7 +2945,8 @@ globalThis.sqlite3InitModule = sqlite3InitModule; T.assert(6 === db.selectValue('select count(*) from kvvfs')); const exp = db.exportToObject(true); T.assert( filename===exp.name, "Broken export filename" ) - .assert( exp?.data?.['kvvfs-sz'] > 0, "Missing kvvfs-sz" ); + .assert( exp?.size > 0, "Missing db size" ) + .assert( exp?.pages?.length > 0, "Missing db pages" ); console.debug("kvvfs to Object:",exp); close(); diff --git a/manifest b/manifest index 4dcc7d9828..ecc13158f2 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\stests\sdemonstrating\sbasic\sconcurrent\suse\sof\skvvfs\sstorage\sand\sthat\skvvfs\sstill\sworks\safter\swiping\sthe\sstorage\sout\sfrom\sunder\sit). -D 2025-11-23T13:33:56.502 +C Initial\swork\son\seliminating\sthe\ssuperfluous-for-transient-storage\skvvfs\skey\sprefixes\sand\simplementing\skvvfs\sdb\simport. +D 2025-11-23T15:38:34.579 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -600,7 +600,7 @@ F ext/wasm/api/sqlite3-api-worker1.c-pp.js 1041dd645e8e821c082b628cd8d9acf70c667 F ext/wasm/api/sqlite3-license-version-header.js 0c807a421f0187e778dc1078f10d2994b915123c1223fe752b60afdcd1263f89 F ext/wasm/api/sqlite3-opfs-async-proxy.js 9654b565b346dc609b75d15337f20acfa7af7d9d558da1afeb9b6d8eaa404966 F ext/wasm/api/sqlite3-vfs-helper.c-pp.js 3f828cc66758acb40e9c5b4dcfd87fd478a14c8fb7f0630264e6c7fa0e57515d -F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js e98bc058013b8fe189e9b38fb9421e6d514b651f4a38c5b6c331aee175cad13d +F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js fb8dd4228767a6cb8079a665e9b9d851e6f509a66852d50d498b9ccf3d98ee8d F ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js 26cb41d5a62f46a106b6371eb00fef02de3cdbfaa51338ba087a45f53028e0d0 F ext/wasm/api/sqlite3-vfs-opfs.c-pp.js 88ce2078267a2d1af57525a32d896295f4a8db7664de0e17e82dc9ff006ed8d3 F ext/wasm/api/sqlite3-vtab-helper.c-pp.js 9097074724172e31e56ce20ccd7482259cf72a76124213cbc9469d757676da86 @@ -647,7 +647,7 @@ F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555 F ext/wasm/test-opfs-vfs.js 1618670e466f424aa289859fe0ec8ded223e42e9e69b5c851f809baaaca1a00c F ext/wasm/tester1-worker.c-pp.html 0e432ec2c0d99cd470484337066e8d27e7aee4641d97115338f7d962bf7b081a F ext/wasm/tester1.c-pp.html 52d88fe2c6f21a046030a36410b4839b632f4424028197a45a3d5669ea724ddb -F ext/wasm/tester1.c-pp.js 003a04f00b5e485055c368aa780a9e3cf6af69ff584bb76c7b68f4ebee1b4dca +F ext/wasm/tester1.c-pp.js 5ff0909c667329a316fd6a5804530b6fae928ca73bc7f7ec78cff030f7829de8 F ext/wasm/tests/opfs/concurrency/index.html 657578a6e9ce1e9b8be951549ed93a6a471f4520a99e5b545928668f4285fb5e F ext/wasm/tests/opfs/concurrency/test.js d08889a5bb6e61937d0b8cbb78c9efbefbf65ad09f510589c779b7cc6a803a88 F ext/wasm/tests/opfs/concurrency/worker.js 0a8c1a3e6ebb38aabbee24f122693f1fb29d599948915c76906681bb7da1d3d2 @@ -717,7 +717,7 @@ F src/notify.c 57c2d1a2805d6dee32acd5d250d928ab94e02d76369ae057dee7d445fd64e878 F src/os.c 509452169d5ea739723e213b8e2481cf0e587f0e88579a912d200db5269f5f6d F src/os.h 1ff5ae51d339d0e30d8a9d814f4b8f8e448169304d83a7ed9db66a65732f3e63 F src/os_common.h 6c0eb8dd40ef3e12fe585a13e709710267a258e2c8dd1c40b1948a1d14582e06 -F src/os_kv.c 0101f8f61456d02afc4d0cb26304a8ebcf343a1be639986486184007007aa26c +F src/os_kv.c d4f6c56d8933010df6a436db973a0888d62ae88aefcbe3091a378adcf208a356 F src/os_setup.h 8efc64eda6a6c2f221387eefc2e7e45fd5a3d5c8337a7a83519ba4fbd2957ae2 F src/os_unix.c 7945ede1e85b2d1b910e1b4af9ba342e964b1e30e79f4176480a60736445cb36 F src/os_win.c a89b501fc195085c7d6c9eec7f5bd782625e94bb2a96b000f4d009703df1083f @@ -2178,8 +2178,8 @@ F tool/version-info.c 33d0390ef484b3b1cb685d59362be891ea162123cea181cb8e6d2cf6dd F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee87c1b31a7 F tool/warnings.sh d924598cf2f55a4ecbc2aeb055c10bd5f48114793e7ba25f9585435da29e7e98 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 393c1beee00ec3b3e98eb385cae0abeb1b34475140c0e8c3d57541d349904436 -R cf7b7a0fd852f7fd31d9360f143096e9 +P cbbb9e61ae81097625488d78c5cbc0065c0acc3c8a7fd819490bc697f9d808c5 +R 80b842cee7b429e5bac8fd43d7b43fce U stephan -Z 284788218e69c476d1bbab2a25fe74dd +Z 30c61370f06e15e0c2021e7507249bff # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 9822f37001..3a8a0654ea 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -cbbb9e61ae81097625488d78c5cbc0065c0acc3c8a7fd819490bc697f9d808c5 +6bc64059410b1868b7a3576e16d03c02e7c007a2be8b313e386eeb2e2a35c258 diff --git a/src/os_kv.c b/src/os_kv.c index c3752c7b33..4a4a486d38 100644 --- a/src/os_kv.c +++ b/src/os_kv.c @@ -194,18 +194,19 @@ static void kvrecordMakeKey( const char *zKeyIn, char *zKeyOut ){ - assert( zClass ); assert( zKeyIn ); assert( zKeyOut ); #ifdef SQLITE_WASM if( !zClass || !zClass[0] ){ - /* The JS bindings use a zClass of NULL for non-local/non-session - instances. */ - sqlite3_snprintf(KVRECORD_KEY_SZ, zKeyOut, "kvvfs-%s", + /* The JS bindings pass a zClass of NULL for non-local/non-session + instances which store _only_ kvvfs state, so they don't need a + key prefix (and having one wastes space). */ + sqlite3_snprintf(KVRECORD_KEY_SZ, zKeyOut, "%s", zKeyIn); return; } #endif + assert( zClass ); sqlite3_snprintf(KVRECORD_KEY_SZ, zKeyOut, "kvvfs-%s-%s", zClass, zKeyIn); }