From 9e5eb9c8cc66320ec01298cc2e76ae6a1e29515a Mon Sep 17 00:00:00 2001 From: drh Date: Sun, 18 Sep 2016 16:08:10 +0000 Subject: [PATCH] Revise the implementation of OP_Once so that it is smaller, faster, and uses less memory. This also fixes an obscure bug introduced 3 days ago by check-in [5990a1bdb4a073]. FossilOrigin-Name: 6bf5ba10d28f1b0a32aa9a560ae3143a1235eadb --- manifest | 32 ++++++++++++++++---------------- manifest.uuid | 2 +- src/expr.c | 2 +- src/global.c | 3 ++- src/main.c | 9 +++++++++ src/sqlite.h.in | 1 + src/sqliteInt.h | 2 +- src/trigger.c | 1 - src/vdbe.c | 42 ++++++++++++++++++++++-------------------- src/vdbe.h | 1 - src/vdbeInt.h | 4 ---- src/vdbeaux.c | 10 ---------- 12 files changed, 53 insertions(+), 56 deletions(-) diff --git a/manifest b/manifest index 0a11d0dc7a..8b2cc6c428 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Minor\senhancement\sto\sthe\sTclKit\sbatch\stool:\sit\sshould\sdownload\sthe\sTclKit\sEXE\salone\swhen\sthe\sTCLKIT_NOENV\senvironment\svariable\sis\sset. -D 2016-09-17T22:46:13.833 +C Revise\sthe\simplementation\sof\sOP_Once\sso\sthat\sit\sis\ssmaller,\sfaster,\sand\suses\nless\smemory.\s\sThis\salso\sfixes\san\sobscure\sbug\sintroduced\s3\sdays\sago\sby\scheck-in\n[5990a1bdb4a073]. +D 2016-09-18T16:08:10.162 F Makefile.in 6fd48ffcf7c2deea7499062d1f3747f986c19678 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc e1aa788e84f926e42239ee167c53f785bedacacd @@ -339,11 +339,11 @@ F src/ctime.c e77f3dc297b4b65c96da78b4ae4272fdfae863d7 F src/date.c 95c9a8d00767e7221a8e9a31f4e913fc8029bf6b F src/dbstat.c 19ee7a4e89979d4df8e44cfac7a8f905ec89b77d F src/delete.c e91a11e0e86a13ce1917ca5ad7cf14c37ba31e59 -F src/expr.c c1de78269522de39f6ad877ab0810a5d48f91f34 +F src/expr.c 5f6c8263b021853aef5a43e920efebd0b94de394 F src/fault.c 160a0c015b6c2629d3899ed2daf63d75754a32bb F src/fkey.c e2be0968c1adc679c87e467aa5b4f167588f38a8 F src/func.c 29cc9acb170ec1387b9f63eb52cd85f8de96c771 -F src/global.c c45ea22aff29334f6a9ec549235ac3357c970015 +F src/global.c 2917bbc488201b791e5f5dd43d8d2a3ccc315da7 F src/hash.c 55b5fb474100cee0b901edaf203e26c970940f36 F src/hash.h ab34c5c54a9e9de2e790b24349ba5aab3dbb4fd4 F src/hwtime.h 747c1bbe9df21a92e9c50f3bbec1de841dc5e5da @@ -351,7 +351,7 @@ F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71 F src/insert.c 427e96ff168b6b187a52c3d06b9f9e41f6ffb212 F src/legacy.c 75d3023be8f0d2b99d60f905090341a03358c58e F src/loadext.c 5d6642d141c07d366e43d359e94ec9de47add41d -F src/main.c 9821bb4d2399bc5a0b8a5dfc06bede2d1520255c +F src/main.c 14fb66eeb2eea477f9e79434fde3cca3aff0ea5c F src/malloc.c 1443d1ad95d67c21d77af7ae3f44678252f0efec F src/mem0.c 6a55ebe57c46ca1a7d98da93aaa07f99f1059645 F src/mem1.c 6919bcf12f221868ea066eec27e579fed95ce98b @@ -388,10 +388,10 @@ F src/resolve.c 3c3cf0dc719cd2a32ab5c1e10c26481dd565492e F src/rowset.c 7b7e7e479212e65b723bf40128c7b36dc5afdfac F src/select.c 244f9cc5e4662987cd2ef5c22d1b7027560f3425 F src/shell.c b80396d2fadce4681397707e30078bf416e1dec2 -F src/sqlite.h.in 46ed821aeed0ba45559fb15597d9a400083154a2 +F src/sqlite.h.in 2683a291ed8db5228024267be6421f0de507b80e F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8 F src/sqlite3ext.h 8648034aa702469afb553231677306cc6492a1ae -F src/sqliteInt.h c9e010a79ab4ed7bdc910a24d8f08f3c6d5f822c +F src/sqliteInt.h 1abb4501bbc28c9badf0ebc1e99bd66585ba04f0 F src/sqliteLimit.h c0373387c287c8d0932510b5547ecde31b5da247 F src/status.c a9e66593dfb28a9e746cba7153f84d49c1ddc4b1 F src/table.c 5226df15ab9179b9ed558d89575ea0ce37b03fc9 @@ -448,16 +448,16 @@ F src/test_wsd.c 41cadfd9d97fe8e3e4e44f61a4a8ccd6f7ca8fe9 F src/threads.c 4ae07fa022a3dc7c5beb373cf744a85d3c5c6c3c F src/tokenize.c 78c8085bc7af1922aa687f0f4bbd716821330de5 F src/treeview.c f51b75a28b377adde9f79bc3deb6c7770bcf97c0 -F src/trigger.c cae81f6c65c4977bc2cbd27d43bfee478710853c +F src/trigger.c 3419bb9862983d84d70735fb4c94b21b934cd0c5 F src/update.c 8179e699dbd45b92934fd02d3d8e3732e8da8802 F src/utf.c 699001c79f28e48e9bcdf8a463da029ea660540c F src/util.c 810ec3f22e2d1b62e66c30fe3621ebdedd23584d F src/vacuum.c 913970b9d86dd6c2b8063ef1af421880f1464ec3 -F src/vdbe.c bd7e890b2d6e2c93c808e5cf0f8e9fb26198d0fc -F src/vdbe.h 67bc551f7faf04c33493892e4b378aada823ed10 -F src/vdbeInt.h c59381049af5c7751a83456c39b80d1a6fde1f9d +F src/vdbe.c cdac9f8c53e2992816809a45910ff33b6122c191 +F src/vdbe.h c044be7050ac6bf596eecc6ab159f5dbc020a3b7 +F src/vdbeInt.h d21f14721dd87975dc9e3bcdbf504f9c098cf611 F src/vdbeapi.c a32d61b7dd05e6890d8fd44d2805f55e2f5ba9f3 -F src/vdbeaux.c 12831f89de66a23d575d129dc1bda543a09a80c7 +F src/vdbeaux.c 66054df369ca76530b6b84e6fce2f847dc99a9b1 F src/vdbeblob.c 3e82a797b60c3b9fed7b8de8c539ca7607874937 F src/vdbemem.c 357caac1a404f37ee6087b17613f92107c13f733 F src/vdbesort.c 91fda3909326860382b0ca8aa251e609c6a9d62c @@ -1525,7 +1525,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 63962e2a927963575be74c3ce11f39f722c07c72 -R a636a72a27c00683275c0e840ed0e183 -U mistachkin -Z c511a2952b921071fe5b6c44565784ab +P baceb988c6f4e93134e6a63173e846af2b73531a +R 305c67fde695fcddb65717c9957658ad +U drh +Z 45869286482c6f2c4387b12de5cab0cb diff --git a/manifest.uuid b/manifest.uuid index 5867b37be4..f8b9f2bf1b 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -baceb988c6f4e93134e6a63173e846af2b73531a \ No newline at end of file +6bf5ba10d28f1b0a32aa9a560ae3143a1235eadb \ No newline at end of file diff --git a/src/expr.c b/src/expr.c index abae812ea5..14bde45bb0 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1974,7 +1974,7 @@ static Select *isCandidateForInOpt(Expr *pX){ */ int sqlite3CodeOnce(Parse *pParse){ Vdbe *v = sqlite3GetVdbe(pParse); /* Virtual machine being coded */ - return sqlite3VdbeAddOp1(v, OP_Once, pParse->nOnce++); + return sqlite3VdbeAddOp0(v, OP_Once); } #ifndef SQLITE_OMIT_SUBQUERY diff --git a/src/global.c b/src/global.c index 76901a89b1..8b676ef66f 100644 --- a/src/global.c +++ b/src/global.c @@ -225,7 +225,8 @@ SQLITE_WSD struct Sqlite3Config sqlite3Config = { #ifndef SQLITE_OMIT_BUILTIN_TEST 0, /* xTestCallback */ #endif - 0 /* bLocaltimeFault */ + 0, /* bLocaltimeFault */ + 0x7ffffffe /* iOnceResetThreshold */ }; /* diff --git a/src/main.c b/src/main.c index c3700cd413..9c161d0ef5 100644 --- a/src/main.c +++ b/src/main.c @@ -3784,6 +3784,15 @@ int sqlite3_test_control(int op, ...){ break; } + /* Set the threshold at which OP_Once counters reset back to zero. + ** By default this is 0xfffffffe (over 2 billion), but that value is + ** too big to test in a reasonable amount of time, so this control is + ** provided to set a small and easily reachable reset value. + */ + case SQLITE_TESTCTRL_ONCE_RESET_THRESHOLD: { + sqlite3GlobalConfig.iOnceResetThreshold = va_arg(ap, int); + break; + } /* sqlite3_test_control(SQLITE_TESTCTRL_VDBE_COVERAGE, xCallback, ptr); ** diff --git a/src/sqlite.h.in b/src/sqlite.h.in index 52b58370dd..778d53d3d6 100644 --- a/src/sqlite.h.in +++ b/src/sqlite.h.in @@ -6717,6 +6717,7 @@ int sqlite3_test_control(int op, ...); #define SQLITE_TESTCTRL_SCRATCHMALLOC 17 #define SQLITE_TESTCTRL_LOCALTIME_FAULT 18 #define SQLITE_TESTCTRL_EXPLAIN_STMT 19 /* NOT USED */ +#define SQLITE_TESTCTRL_ONCE_RESET_THRESHOLD 19 #define SQLITE_TESTCTRL_NEVER_CORRUPT 20 #define SQLITE_TESTCTRL_VDBE_COVERAGE 21 #define SQLITE_TESTCTRL_BYTEORDER 22 diff --git a/src/sqliteInt.h b/src/sqliteInt.h index dbb18a7458..ebae7e54ab 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -2896,7 +2896,6 @@ struct Parse { int nTab; /* Number of previously allocated VDBE cursors */ int nMem; /* Number of memory cells used so far */ int nSet; /* Number of sets used so far */ - int nOnce; /* Number of OP_Once instructions so far */ int nOpAlloc; /* Number of slots allocated for Vdbe.aOp[] */ int szOpAlloc; /* Bytes of memory space allocated for Vdbe.aOp[] */ int iFixedOp; /* Never back out opcodes iFixedOp-1 or earlier */ @@ -3230,6 +3229,7 @@ struct Sqlite3Config { int (*xTestCallback)(int); /* Invoked by sqlite3FaultSim() */ #endif int bLocaltimeFault; /* True to fail localtime() calls */ + int iOnceResetThreshold; /* When to reset OP_Once counters */ }; /* diff --git a/src/trigger.c b/src/trigger.c index 5c80560114..7eb14bcf8d 100644 --- a/src/trigger.c +++ b/src/trigger.c @@ -879,7 +879,6 @@ static TriggerPrg *codeRowTrigger( } pProgram->nMem = pSubParse->nMem; pProgram->nCsr = pSubParse->nTab; - pProgram->nOnce = pSubParse->nOnce; pProgram->token = (void *)pTrigger; pPrg->aColmask[0] = pSubParse->oldmask; pPrg->aColmask[1] = pSubParse->newmask; diff --git a/src/vdbe.c b/src/vdbe.c index 4aa26d006e..51092cfedb 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -2332,22 +2332,18 @@ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ /* Opcode: Once P1 P2 * * * ** -** Check the "once" flag number P1. If it is set, jump to instruction P2. -** Otherwise, set the flag and fall through to the next instruction. -** In other words, this opcode causes all following opcodes up through P2 -** (but not including P2) to run just once and to be skipped on subsequent -** times through the loop. -** -** All "once" flags are initially cleared whenever a prepared statement -** first begins to run. +** If the P1 value is equal to the P1 value on the OP_Init opcode at +** instruction 0, then jump to P2. If the two P1 values differ, then +** set the P1 value on this opcode to equal the P1 value on the OP_Init +** and fall through. */ case OP_Once: { /* jump */ - assert( pOp->p1nOnceFlag ); - VdbeBranchTaken(p->aOnceFlag[pOp->p1]!=0, 2); - if( p->aOnceFlag[pOp->p1] ){ + assert( p->aOp[0].opcode==OP_Init ); + VdbeBranchTaken(p->aOp[0].p1==pOp->p1, 2); + if( p->aOp[0].p1==pOp->p1 ){ goto jump_to_p2; }else{ - p->aOnceFlag[pOp->p1] = 1; + pOp->p1 = p->aOp[0].p1; } break; } @@ -5790,8 +5786,7 @@ case OP_Program: { /* jump */ if( pProgram->nCsr==0 ) nMem++; nByte = ROUND8(sizeof(VdbeFrame)) + nMem * sizeof(Mem) - + pProgram->nCsr * sizeof(VdbeCursor *) - + pProgram->nOnce * sizeof(u8); + + pProgram->nCsr * sizeof(VdbeCursor *); pFrame = sqlite3DbMallocZero(db, nByte); if( !pFrame ){ goto no_mem; @@ -5811,8 +5806,6 @@ case OP_Program: { /* jump */ pFrame->aOp = p->aOp; pFrame->nOp = p->nOp; pFrame->token = pProgram->token; - pFrame->aOnceFlag = p->aOnceFlag; - pFrame->nOnceFlag = p->nOnceFlag; #ifdef SQLITE_ENABLE_STMT_SCANSTATUS pFrame->anExec = p->anExec; #endif @@ -5846,13 +5839,10 @@ case OP_Program: { /* jump */ p->apCsr = (VdbeCursor **)&aMem[p->nMem]; p->aOp = aOp = pProgram->aOp; p->nOp = pProgram->nOp; - p->aOnceFlag = (u8 *)&p->apCsr[p->nCursor]; - p->nOnceFlag = pProgram->nOnce; #ifdef SQLITE_ENABLE_STMT_SCANSTATUS p->anExec = 0; #endif pOp = &aOp[-1]; - memset(p->aOnceFlag, 0, p->nOnceFlag); break; } @@ -6819,7 +6809,7 @@ case OP_MaxPgcnt: { /* out2 */ #endif -/* Opcode: Init * P2 * P4 * +/* Opcode: Init P1 P2 * P4 * ** Synopsis: Start at P2 ** ** Programs contain a single instance of this opcode as the very first @@ -6830,9 +6820,13 @@ case OP_MaxPgcnt: { /* out2 */ ** Or if P4 is blank, use the string returned by sqlite3_sql(). ** ** If P2 is not zero, jump to instruction P2. +** +** Increment the value of P1 so that OP_Once opcodes will jump the +** first time they are evaluated for this run. */ case OP_Init: { /* jump */ char *zTrace; + int i; /* If the P4 argument is not NULL, then it must be an SQL comment string. ** The "--" string is broken up to prevent false-positives with srcck1.c. @@ -6844,6 +6838,7 @@ case OP_Init: { /* jump */ ** sqlite3_expanded_sql(P) otherwise. */ assert( pOp->p4.z==0 || strncmp(pOp->p4.z, "-" "- ", 3)==0 ); + assert( pOp==p->aOp ); /* Always instruction 0 */ #ifndef SQLITE_OMIT_TRACE if( (db->mTrace & (SQLITE_TRACE_STMT|SQLITE_TRACE_LEGACY))!=0 @@ -6881,6 +6876,13 @@ case OP_Init: { /* jump */ #endif /* SQLITE_DEBUG */ #endif /* SQLITE_OMIT_TRACE */ assert( pOp->p2>0 ); + if( pOp->p1>=sqlite3GlobalConfig.iOnceResetThreshold ){ + for(i=1; inOp; i++){ + if( p->aOp[i].opcode==OP_Once ) p->aOp[i].p1 = 0; + } + pOp->p1 = 0; + } + pOp->p1++; goto jump_to_p2; } diff --git a/src/vdbe.h b/src/vdbe.h index a7bc84cea5..ecb2b0e3f4 100644 --- a/src/vdbe.h +++ b/src/vdbe.h @@ -88,7 +88,6 @@ struct SubProgram { int nOp; /* Elements in aOp[] */ int nMem; /* Number of memory cells required */ int nCsr; /* Number of cursors required */ - int nOnce; /* Number of OP_Once instructions */ void *token; /* id that may be used to recursive triggers */ SubProgram *pNext; /* Next sub-program already visited */ }; diff --git a/src/vdbeInt.h b/src/vdbeInt.h index ecb4fc5f06..b1d7717a0b 100644 --- a/src/vdbeInt.h +++ b/src/vdbeInt.h @@ -157,7 +157,6 @@ struct VdbeFrame { Op *aOp; /* Program instructions for parent frame */ i64 *anExec; /* Event counters from parent frame */ Mem *aMem; /* Array of memory cells for parent frame */ - u8 *aOnceFlag; /* Array of OP_Once flags for parent frame */ VdbeCursor **apCsr; /* Array of Vdbe cursors for parent frame */ void *token; /* Copy of SubProgram.token */ i64 lastRowid; /* Last insert rowid (sqlite3.lastRowid) */ @@ -166,7 +165,6 @@ struct VdbeFrame { int pc; /* Program Counter in parent (calling) frame */ int nOp; /* Size of aOp array */ int nMem; /* Number of entries in aMem */ - int nOnceFlag; /* Number of entries in aOnceFlag */ int nChildMem; /* Number of memory cells for child frame */ int nChildCsr; /* Number of cursors for child frame */ int nChange; /* Statement changes (Vdbe.nChange) */ @@ -410,8 +408,6 @@ struct Vdbe { int nFrame; /* Number of frames in pFrame list */ u32 expmask; /* Binding to these vars invalidates VM */ SubProgram *pProgram; /* Linked list of all sub-programs used by VM */ - int nOnceFlag; /* Size of array aOnceFlag[] */ - u8 *aOnceFlag; /* Flags for OP_Once */ AuxData *pAuxData; /* Linked list of auxdata allocations */ #ifdef SQLITE_ENABLE_STMT_SCANSTATUS i64 *anExec; /* Number of times each op has been executed */ diff --git a/src/vdbeaux.c b/src/vdbeaux.c index 56f00efcfd..838a8f0446 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -1869,7 +1869,6 @@ void sqlite3VdbeMakeReady( int nMem; /* Number of VM memory registers */ int nCursor; /* Number of cursors required */ int nArg; /* Number of arguments in subprograms */ - int nOnce; /* Number of OP_Once instructions */ int n; /* Loop counter */ struct ReusableSpace x; /* Reusable bulk memory */ @@ -1884,8 +1883,6 @@ void sqlite3VdbeMakeReady( nMem = pParse->nMem; nCursor = pParse->nTab; nArg = pParse->nMaxArg; - nOnce = pParse->nOnce; - if( nOnce==0 ) nOnce = 1; /* Ensure at least one byte in p->aOnceFlag[] */ /* Each cursor uses a memory cell. The first cursor (cursor 0) can ** use aMem[0] which is not otherwise used by the VDBE program. Allocate @@ -1932,7 +1929,6 @@ void sqlite3VdbeMakeReady( p->aVar = allocSpace(&x, p->aVar, nVar*sizeof(Mem)); p->apArg = allocSpace(&x, p->apArg, nArg*sizeof(Mem*)); p->apCsr = allocSpace(&x, p->apCsr, nCursor*sizeof(VdbeCursor*)); - p->aOnceFlag = allocSpace(&x, p->aOnceFlag, nOnce); #ifdef SQLITE_ENABLE_STMT_SCANSTATUS p->anExec = allocSpace(&x, p->anExec, p->nOp*sizeof(i64)); #endif @@ -1942,7 +1938,6 @@ void sqlite3VdbeMakeReady( }while( !db->mallocFailed ); p->nCursor = nCursor; - p->nOnceFlag = nOnce; if( p->aVar ){ p->nVar = (ynVar)nVar; for(n=0; nanExec = pFrame->anExec; #endif - v->aOnceFlag = pFrame->aOnceFlag; - v->nOnceFlag = pFrame->nOnceFlag; v->aOp = pFrame->aOp; v->nOp = pFrame->nOp; v->aMem = pFrame->aMem; @@ -2569,11 +2562,8 @@ int sqlite3VdbeHalt(Vdbe *p){ ** one, or the complete transaction if there is no statement transaction. */ - assert( p->aOnceFlag!=0 || db->mallocFailed ); if( db->mallocFailed ){ p->rc = SQLITE_NOMEM_BKPT; - }else{ - memset(p->aOnceFlag, 0, p->nOnceFlag); } closeAllCursors(p); if( p->magic!=VDBE_MAGIC_RUN ){ -- 2.47.2