paulhorn/sqlite
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Replace an alloc/free on every read of a JS kvvfs key with a one-time alloc and a long paragraph explaining why we have to leak it.

Browse Source 
FossilOrigin-Name: 66fb9978f0a63887d214e895343adedfa46ee7aefccb8389d6d3af727e0a65ea
This commit is contained in:
stephan
2025-11-23 11:28:27 +00:00
parent 44fdf80c28
commit 48c8405f99
4 changed files with 92 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ext/wasm/api/sqlite3-api-prologue.js
+1 -1
View File
@@ -892,7 +892,7 @@ globalThis.sqlite3ApiBootstrap = async function sqlite3ApiBootstrap(
Like this.alloc.impl(), this.realloc.impl() is a direct binding
to the underlying realloc() implementation which does not throw
exceptions, instead returning 0 on allocation error.
exceptions, instead returning 0 (or 0n) on allocation error.
*/
realloc: undefined/*installed later*/,
ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js
+83 -64
View File
@@ -308,18 +308,16 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
const pVfs = new capi.sqlite3_vfs(kvvfsMethods.$pVfs);
const pIoDb = new capi.sqlite3_io_methods(kvvfsMethods.$pIoDb);
const pIoJrnl = new capi.sqlite3_io_methods(kvvfsMethods.$pIoJrnl);
const recordHandler = Object.create(null);
const recordHandler =
Object.create(null)/** helper for some vfs
routines. Populated later. */;
/**
Implementations for members of the object referred to by
sqlite3__wasm_kvvfs_methods(). We swap out the native
sqlite3__wasm_kvvfs_methods(). We swap out some native
implementations with these, which use JS Storage for their
backing store.
The interface docs for these methods are in
src/os_kv.c:kvrecordRead(), kvrecordWrite(), and
kvrecordDelete().
*/
sqlite3_kvvfs_methods.override = {
const methodOverrides = {
/**
sqlite3_kvvfs_methods's member methods. These perform the
@@ -327,11 +325,21 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
kvvfs. In the native impl these write each db page to a
separate file. This impl stores each db page as a single
record in a Storage object which is mapped to zClass.
A db's size is stored in a record named kvvfs[-storagename]-sz
and the journal is stored in kvvfs[-storagename]-jrnl. The
[-storagename] part is a remnant of the native impl (so that
it has unique filenames per db) and is only used for
localStorage and sessionStorage. We elide that part (to save
space) from other storage objects but retain it on those two
to avoid invalidating pre-version-2 session/localStorage dbs.
The interface docs for these methods are in src/os_kv.c's
kvrecordRead(), kvrecordWrite(), and kvrecordDelete().
*/
recordHandler: {
xRcrdRead: (zClass, zKey, zBuf, nBuf)=>{
const stack = pstack.pointer,
astack = wasm.scopedAllocPush();
const stack = pstack.pointer /* keyForStorage() allocs from here */;
try{
const store = storageForZClass(zClass);
if( 0 ){
@@ -354,31 +362,41 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
wasm.poke(zBuf, 0);
return nV;
}
util.assert( nBuf>nV, "xRcrdRead Input buffer is too small" );
if( 0 ){
debug("xRcrdRead", nBuf, zClass, wasm.cstrToJs(zClass),
wasm.cstrToJs(zKey), nV, jV, store);
}
const zV = wasm.scopedAllocCString(jV)
/* TODO: allocate a single 128kb buffer (largest page
size) for reuse here, or maybe even preallocate
it somewhere in sqlite3__wasm_kvvfs_...(). */;
if(nBuf > nV + 1) nBuf = nV + 1;
wasm.heap8u().copyWithin(
Number(zBuf), Number(zV), wasm.ptr.addn(zV, nBuf,- 1)
const zV = cache.keybuf.mem ??= wasm.alloc.impl(cache.keybuf.n)
/* We leak this one-time alloc because we've no better
option. sqlite3_vfs does not have a finalizer, so
we've no place to hook in the cleanup. We "could"
extend sqlite3_shutdown() to have a cleanup list for
stuff like this but (A) that function is never used in
JS and (B) its cleanup would leave cache.keybuf
pointing to stale memory, so if the library were used
after sqlite3_shutdown() then we'd corrupt memory. */;
if( !zV ) return -3 /*OOM*/;
const heap = wasm.heap8();
let i;
for(i = 0; i < nV; ++i){
heap[wasm.ptr.add(zV,i)] = jV.codePointAt(i) & 0xFF;
}
heap.copyWithin(
Number(zBuf), Number(zV), wasm.ptr.addn(zV, i)
);
wasm.poke(wasm.ptr.add(zBuf, nBuf), 0);
heap[wasm.ptr.add(zBuf, nV)] = 0;
return nBuf;
}catch(e){
error("kvrecordRead()",e);
return -2;
}finally{
pstack.restore(stack);
wasm.scopedAllocPop(astack);
}
},
xRcrdWrite: (zClass, zKey, zData)=>{
const stack = pstack.pointer;
const stack = pstack.pointer /* keyForStorage() allocs from here */;
try {
const store = storageForZClass(zClass);
const zXKey = keyForStorage(store, zClass, zKey);
@@ -397,7 +415,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
},
xRcrdDelete: (zClass, zKey)=>{
const stack = pstack.pointer;
const stack = pstack.pointer /* keyForStorage() allocs from here */;
try {
const store = storageForZClass(zClass);
const zXKey = keyForStorage(store, zClass, zKey);
@@ -415,8 +433,8 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
/**
Override certain operations of the underlying sqlite3_vfs and
two sqlite3_io_methods instances so that we can tie Storage
objects to db names.
the two sqlite3_io_methods instances so that we can tie
Storage objects to db names.
*/
vfs:{
/* sqlite3_kvvfs_methods::pVfs's methods */
@@ -427,44 +445,42 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
if( n > kvvfsMethods.$nKeySize - 8 /*"-journal"*/ - 1 ){
warn("file name is too long:", wasm.cstrToJs(zName));
return capi.SQLITE_RANGE;
}else{
wasm.poke32(pOutFlags, flags | sqlite3.SQLITE_OPEN_CREATE);
}
//wasm.poke32(pOutFlags, flags | sqlite3.SQLITE_OPEN_CREATE);
const rc = originalMethods.vfs.xOpen(pProtoVfs, zName, pProtoFile,
flags, pOutFlags);
if( 0==rc ){
const f = new KVVfsFile(pProtoFile);
util.assert(f.$zClass, "Missing f.$zClass");
const jzClass = wasm.cstrToJs(zName);//f.$zClass);
let s = cache.jzClassToStorage[jzClass];
//debug("xOpen", jzClass, s);
if( s ){
++s.refc;
}else{
/* TODO: a url flag which tells it to keep the storage
around forever so that future xOpen()s get the same
Storage-ish objects. We can accomplish that by
simply increasing the refcount once more. */
util.assert( !f.$isJournal, "Opening a journal before its db? "+jzClass );
const other = f.$isJournal
? jzClass.replace(cache.rxJournalSuffix,'')
: jzClass + '-journal';
s = cache.jzClassToStorage[jzClass]
= cache.jzClassToStorage[other]
= Object.assign(Object.create(null),{
jzClass: jzClass,
refc: 1/* if this is a db-open, the journal open
will follow soon enough and bump the
refcount. If we start at 2 here, that
pending open will increment it again. */,
s: new TransientStorage
});
debug("xOpen installed storage handles [",
jzClass, other,"]", s);
}
pFileHandles.set(pProtoFile, {s,f,n:jzClass});
if( rc ) return rc;
const f = new KVVfsFile(pProtoFile);
util.assert(f.$zClass, "Missing f.$zClass");
const jzClass = wasm.cstrToJs(zName);//f.$zClass);
let s = cache.jzClassToStorage[jzClass];
//debug("xOpen", jzClass, s);
if( s ){
++s.refc;
}else{
/* TODO: a url flag which tells it to keep the storage
around forever so that future xOpen()s get the same
Storage-ish objects. We can accomplish that by
simply increasing the refcount once more. */
util.assert( !f.$isJournal, "Opening a journal before its db? "+jzClass );
const other = f.$isJournal
? jzClass.replace(cache.rxJournalSuffix,'')
: jzClass + '-journal';
s = cache.jzClassToStorage[jzClass]
= cache.jzClassToStorage[other]
= Object.assign(Object.create(null),{
jzClass: jzClass,
refc: 1/* if this is a db-open, the journal open
will follow soon enough and bump the
refcount. If we start at 2 here, that
pending open will increment it again. */,
s: new TransientStorage
});
debug("xOpen installed storage handles [",
jzClass, other,"]", s);
}
return rc;
pFileHandles.set(pProtoFile, {s,f,n:jzClass});
return 0;
}catch(e){
warn("xOpen:",e);
return capi.SQLITE_ERROR;
@@ -500,7 +516,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
mystery.
*/
wasm.poke32(pResOut, 0);
return 0;
if( 1 ) return 0;
if( 1 ){
/* This is fundamentally exactly what the native impl does. */
const jzName = wasm.cstrToJs(zPath);
@@ -628,20 +644,23 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
xDeviceCharacteristics: true
//#endif
}
}/*sqlite3_kvvfs_methods.override*/;
}/*methodOverrides*/;
const ov = sqlite3_kvvfs_methods.override;
debug("pVfs and friends", pVfs, pIoDb, pIoJrnl,
kvvfsMethods, capi.sqlite3_file.structInfo,
KVVfsFile.structInfo);
try {
for(const e of Object.entries(ov.recordHandler)){
cache.keybuf = Object.create(null);
cache.keybuf.n = kvvfsMethods.$nBufferSize;
util.assert( cache.keybuf.n>1024*129, "Key buffer is not large enough"
/* Native is SQLITE_KVOS_SZ is 133073 as of this writing */ );
for(const e of Object.entries(methodOverrides.recordHandler)){
// Overwrite kvvfsMethods's callbacks
recordHandler[e[0]] = e[1];
kvvfsMethods[kvvfsMethods.memberKey(e[0])] =
wasm.installFunction(kvvfsMethods.memberSignature(e[0]), e[1]);
}
for(const e of Object.entries(ov.vfs)){
for(const e of Object.entries(methodOverrides.vfs)){
// Overwrite some pVfs entries and stash the original impls
const k = e[0], f = e[1], km = pVfs.memberKey(k),
member = pVfs.structInfo.members[k]
@@ -650,14 +669,14 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
|| util.toss("Missing native pVfs[",km,"]");
pVfs[km] = wasm.installFunction(member.signature, f);
}
for(const e of Object.entries(ov.ioDb)){
for(const e of Object.entries(methodOverrides.ioDb)){
// Similar treatment for pVfs.$pIoDb a.k.a. pIoDb...
const k = e[0], f = e[1], km = pIoDb.memberKey(k);
originalMethods.ioDb[k] = wasm.functionEntry(pIoDb[km])
|| util.toss("Missing native pIoDb[",km,"]");
pIoDb[km] = wasm.installFunction(pIoDb.memberSignature(k), f);
}
for(const e of Object.entries(ov.ioJrnl)){
for(const e of Object.entries(methodOverrides.ioJrnl)){
// Similar treatment for pVfs.$pIoJrnl a.k.a. pIoJrnl...
const k = e[0], f = e[1], km = pIoJrnl.memberKey(k);
originalMethods.ioJrnl[k] = wasm.functionEntry(pIoJrnl[km])
@@ -675,7 +694,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
pIoDb.dispose();
pIoJrnl.dispose();
}
}/*method overrides*/
}/*native method overrides*/
if(sqlite3?.oo1?.DB){
/**
manifest
+7 -7
View File
@@ -1,5 +1,5 @@
C This\scombination\sof\skvvfs\scallbacks\sseems\sto\swork\sokay\sfor\sboth\spersistent\sand\stransient\sstorage,\sbut\sit's\sunclear\swhy/how,\sas\sxAccess()\sdoes\snot\sappear\sto\sbe\sdoing\sanything\suseful\s(and\smisbehaves\swhen\stold\sto\sreport\sthat\sthe\sstorage\sobject\sin\squestion\sexists).\sThese\stests\sfail\sin\s64-bit\sbuilds\sfor\sas-yet-unknown\sreasons\s(other\stests\scontinue\sto\spass\sin\s64-bit).
D 2025-11-22T19:21:25.670
C Replace\san\salloc/free\son\severy\sread\sof\sa\sJS\skvvfs\skey\swith\sa\sone-time\salloc\sand\sa\slong\sparagraph\sexplaining\swhy\swe\shave\sto\sleak\sit.
D 2025-11-23T11:28:27.602
F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x
F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
@@ -595,12 +595,12 @@ F ext/wasm/api/post-js-header.js d24bd0d065f3489c8b78ddf3ead6321e5d047187a162cd5
F ext/wasm/api/pre-js.c-pp.js ad2546290e0c8ce5ca2081bff6e85cc25eeb904a3303921f1184290a7ff1b32f
F ext/wasm/api/sqlite3-api-glue.c-pp.js 9eaed1801be392f6687aa7da8e3a5a41d03de19993d8fe62ee6c52617eab4985
F ext/wasm/api/sqlite3-api-oo1.c-pp.js 7d8850f754b4a9aecd5a4a92a357e05f720cd7f5cf962909343b40c914237256
F ext/wasm/api/sqlite3-api-prologue.js 29bc41d0f9fbca946fc60681fb79901b3224988b5b978a47620595bbe0e4cf84
F ext/wasm/api/sqlite3-api-prologue.js 2ac62b41dd8d66859c86a6af126690851e5e557dad61ef59692389762c9bd2ed
F ext/wasm/api/sqlite3-api-worker1.c-pp.js 1041dd645e8e821c082b628cd8d9acf70c667430f9d45167569633ffc7567938
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 bd461cf2e1c35aab71ce43906b514d9f5bb348cbbcadf0bac46d53a3ab84af66
F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js 6a7007c26361b9d9b74d0740c61eea5d93253675824422b100be38b9cbe25073
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
@@ -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 0b53be562f1e1a5b20ffe8d72df64e753a8d759b580d949a0f32409144769bb0
R 18ff16459471e0fa48753f413430772f
P 69e591d0054218ead789cee199e5258f1c378a89e4b7b0e38fe74e834e23156b
R 59e8541a0d2f7929ff70c46115b7160f
U stephan
Z 100d31fa602c6a076823477a2cb4e392
Z dc6a511f30ab2f711394f8b6dd32e86b
# Remove this line to create a well-formed Fossil manifest.
manifest.uuid
+1 -1
View File
@@ -1 +1 @@
69e591d0054218ead789cee199e5258f1c378a89e4b7b0e38fe74e834e23156b
66fb9978f0a63887d214e895343adedfa46ee7aefccb8389d6d3af727e0a65ea