From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B6B3326382 for ; Tue, 10 Jul 2018 13:08:20 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 39Y5xWNrbISr for ; Tue, 10 Jul 2018 13:08:20 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4AB1926337 for ; Tue, 10 Jul 2018 13:08:20 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v2 4/4] sql: remove OP_LoadPtr Date: Tue, 10 Jul 2018 20:08:11 +0300 Message-Id: <8e1537ec82992142f31fe561102fdbce690d1754.1531242355.git.kshcherbatov@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: korablev@tarantool.org, Vladislav Shpilevoy From: Vladislav Shpilevoy This opcode was used when Vdbe had to store key_def in P4 for OP_OpenRead/Write, so P4 was occupied and OP_LoadPtr was used to store space pointer in another opcode. But now P4 is free for OpenRead/Write and space pointer can be stored here. Alongside, some useless key_def duplications are removed. --- src/box/sql/analyze.c | 8 ++---- src/box/sql/build.c | 20 +++++---------- src/box/sql/delete.c | 15 ++--------- src/box/sql/insert.c | 18 +++---------- src/box/sql/pragma.c | 39 ++++++++++++++-------------- src/box/sql/sqliteInt.h | 5 ---- src/box/sql/vdbe.c | 68 +++++-------------------------------------------- 7 files changed, 40 insertions(+), 133 deletions(-) diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index 067a86e..699a4e8 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -794,7 +794,6 @@ analyzeOneTable(Parse * pParse, /* Parser context */ int iTabCur; /* Table cursor */ Vdbe *v; /* The virtual machine being built up */ int i; /* Loop counter */ - int space_ptr_reg = iMem++; int regStat4 = iMem++; /* Register to hold Stat4Accum object */ int regChng = iMem++; /* Index of changed index field */ int regKey = iMem++; /* Key argument passed to stat_push() */ @@ -894,11 +893,8 @@ analyzeOneTable(Parse * pParse, /* Parser context */ space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum)); int idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum); assert(space != NULL); - sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, - (void*)space, P4_SPACEPTR); - sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, idx_id, - space_ptr_reg); - sql_vdbe_set_p4_key_def(pParse, pIdx); + sqlite3VdbeAddOp4(v, OP_OpenRead, iIdxCur, idx_id, 0, + (void *) space, P4_SPACEPTR); VdbeComment((v, "%s", pIdx->zName)); /* Invoke the stat_init() function. The arguments are: diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 5f7a35a..26af0fe 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1108,12 +1108,8 @@ vdbe_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id, struct space *space) { assert(space != NULL); - struct Vdbe *vdbe = parse_context->pVdbe; - int space_ptr_reg = ++parse_context->nMem; - sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space, - P4_SPACEPTR); - return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, index_id, - space_ptr_reg); + return sqlite3VdbeAddOp4(parse_context->pVdbe, OP_OpenWrite, cursor, + index_id, 0, (void *) space, P4_SPACEPTR); } /* * Generate code that will increment the schema cookie. @@ -2961,7 +2957,6 @@ sql_create_index(struct Parse *parse, struct Token *token, Vdbe *v; char *zStmt; int iCursor = parse->nTab++; - int index_space_ptr_reg = parse->nTab++; int iSpaceId, iIndexId, iFirstSchemaCol; v = sqlite3GetVdbe(parse); @@ -2969,12 +2964,9 @@ sql_create_index(struct Parse *parse, struct Token *token, goto exit_create_index; sql_set_multi_write(parse, true); - - - sqlite3VdbeAddOp2(v, OP_SIDtoPtr, BOX_INDEX_ID, - index_space_ptr_reg); - sqlite3VdbeAddOp4Int(v, OP_OpenWrite, iCursor, 0, - index_space_ptr_reg, 6); + sqlite3VdbeAddOp4(v, OP_OpenWrite, iCursor, 0, 0, + (void *)space_by_id(BOX_INDEX_ID), + P4_SPACEPTR); sqlite3VdbeChangeP5(v, OPFLAG_SEEKEQ); /* @@ -3943,7 +3935,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id, int cursor = parser->nTab++; vdbe_emit_open_cursor(parser, cursor, index_id, space_by_id(space_id)); - int name_reg = parser->nMem++; + int name_reg = ++parser->nMem; int label = sqlite3VdbeAddOp4(v, OP_String8, 0, name_reg, 0, name, P4_DYNAMIC); sqlite3VdbeAddOp4Int(v, cond_opcode, cursor, label + 3, name_reg, 1); diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index d1c5935..ca1e77d 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -335,19 +335,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, iAddrOnce = sqlite3VdbeAddOp0(v, OP_Once); VdbeCoverage(v); } - - int space_ptr_reg = ++parse->nMem; - sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, - (void *)space, P4_SPACEPTR); - - sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, 0, - space_ptr_reg); - struct key_def *def = key_def_dup(pk_def); - if (def == NULL) { - sqlite3OomFault(parse->db); - goto delete_from_cleanup; - } - sqlite3VdbeAppendP4(v, def, P4_KEYDEF); + sqlite3VdbeAddOp4(v, OP_OpenWrite, tab_cursor, 0, 0, + (void *) space, P4_SPACEPTR); VdbeComment((v, "%s", space->index[0]->def->name)); if (one_pass == ONEPASS_MULTI) diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 62b85d5..6aede16 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -204,13 +204,8 @@ vdbe_has_table_read(struct Parse *parser, const struct Table *table) * and Write cursors. */ if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) { - assert(i > 1); - struct VdbeOp *space_var_op = - sqlite3VdbeGetOp(v, i - 1); - assert(space_var_op != NULL); - assert(space_var_op->opcode == OP_LoadPtr); - struct space *space = space_var_op->p4.space; - + assert(op->p4type == P4_SPACEPTR); + struct space *space = op->p4.space; if (space->def->id == table->def->id) return true; @@ -1581,10 +1576,6 @@ sqlite3OpenTableAndIndices(Parse * pParse, /* Parsing context */ *piIdxCur = iBase; struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pTab->tnum)); assert(space != NULL); - int space_ptr_reg = ++pParse->nMem; - sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space, - P4_SPACEPTR); - /* One iteration of this cycle adds OpenRead/OpenWrite which * opens cursor for current index. * @@ -1633,9 +1624,8 @@ sqlite3OpenTableAndIndices(Parse * pParse, /* Parsing context */ if (aToOpen == 0 || aToOpen[i + 1]) { int idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum); - sqlite3VdbeAddOp3(v, op, iIdxCur, idx_id, - space_ptr_reg); - sql_vdbe_set_p4_key_def(pParse, pIdx); + sqlite3VdbeAddOp4(v, op, iIdxCur, idx_id, 0, + (void *) space, P4_SPACEPTR); sqlite3VdbeChangeP5(v, p5); VdbeComment((v, "%s", pIdx->zName)); } diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index ca119e0..8ff5bbc 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -678,29 +678,28 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ x = sqlite3FkLocateIndex(pParse, pParent, pFK, &pIdx, 0); - if (x == 0) { - if (pIdx == 0) { - sqlite3OpenTable(pParse, - i, - pParent, - OP_OpenRead); - } else { - int idx_id = - SQLITE_PAGENO_TO_INDEXID( - pIdx-> - tnum); - sqlite3VdbeAddOp3(v, - OP_OpenRead, - i, - idx_id, - 0); - sql_vdbe_set_p4_key_def(pParse, - pIdx); - } - } else { + if (x != 0) { k = 0; break; } + if (pIdx == NULL) { + sqlite3OpenTable(pParse, i, + pParent, + OP_OpenRead); + continue; + } + struct space *space = + space_cache_find(pIdx->pTable-> + def->id); + int idx_id = + SQLITE_PAGENO_TO_INDEXID(pIdx-> + tnum); + assert(space != NULL); + sqlite3VdbeAddOp4(v, OP_OpenRead, i, + idx_id, 0, + (void *) space, + P4_SPACEPTR); + } assert(pParse->nErr > 0 || pFK == 0); if (pFK) diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 4a1eef0..18bf949 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -3008,11 +3008,6 @@ struct Parse { #define OPFLAG_NOOP_IF_NULL 0x02 /* OP_FCopy: if source register is NULL * then do nothing */ -#define OPFLAG_FRESH_PTR 0x20 /* OP_Open**: set if space pointer - * comes from OP_SIDtoPtr, i.e. it - * is fresh, even in case schema - * changes previously. - */ /* * Each trigger present in the database schema is stored as an diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index a0eb41b..49a17b9 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1077,19 +1077,6 @@ case OP_Int64: { /* out2 */ break; } -/* Opcode: LoadPtr * P2 * P4 * - * Synopsis: r[P2] = P4 - * - * P4 is a generic or space pointer. Copy it into register P2. - */ -case OP_LoadPtr: { - pOut = out2Prerelease(p, pOp); - assert(pOp->p4type == P4_PTR || pOp->p4type == P4_SPACEPTR ); - pOut->u.p = pOp->p4.space; - pOut->flags = MEM_Ptr; - break; -} - #ifndef SQLITE_OMIT_FLOATING_POINT /* Opcode: Real * P2 * P4 * * Synopsis: r[P2]=P4 @@ -3137,26 +3124,15 @@ case OP_SetCookie: { } /* Opcode: OpenRead P1 P2 P3 P4 P5 - * Synopsis: index id = P2, space ptr = P3 + * Synopsis: index id = P2, space ptr = P4 * - * Open a cursor for a space specified by pointer in P3 and index + * Open a cursor for a space specified by pointer in P4 and index * id in P2. Give the new cursor an identifier of P1. The P1 * values need not be contiguous but all P1 values should be * small integers. It is an error for P1 to be negative. - * - * The P4 value may be a pointer to a key_def structure. - * If it is a pointer to a key_def structure, then said structure - * defines the content and collatining sequence of the index - * being opened. Otherwise, P4 is NULL. - * - * If schema has changed since compile time, VDBE ends execution - * with appropriate error message. The only exception is - * when P5 is set to OPFLAG_FRESH_PTR, which means that - * space pointer has been fetched in runtime right before - * this opcode. */ /* Opcode: ReopenIdx P1 P2 P3 P4 P5 - * Synopsis: index id = P2, space ptr = P3 + * Synopsis: index id = P2, space ptr = P4 * * The ReopenIdx opcode works exactly like OpenRead except that * it first checks to see if the cursor on P1 is already open @@ -3166,7 +3142,7 @@ case OP_SetCookie: { * The ReopenIdx opcode may only be used with P5 == 0. */ /* Opcode: OpenWrite P1 P2 P3 P4 P5 - * Synopsis: index id = P2, space ptr = P3 + * Synopsis: index id = P2, space ptr = P4 * * For now, OpenWrite is an alias for OpenRead. * It exists just due legacy reasons and should be removed: @@ -3182,9 +3158,7 @@ case OP_ReopenIdx: { assert(pOp->p5==0 || pOp->p5==OPFLAG_SEEKEQ); pCur = p->apCsr[pOp->p1]; p2 = pOp->p2; - pIn3 = &aMem[pOp->p3]; - assert(pIn3->flags & MEM_Ptr); - if (pCur && pCur->uc.pCursor->space == (struct space *) pIn3->u.p && + if (pCur && pCur->uc.pCursor->space == pOp->p4.space && pCur->uc.pCursor->index->def->iid == (uint32_t)p2) goto open_cursor_set_hints; /* If the cursor is not currently open or is open on a different @@ -3194,13 +3168,7 @@ case OP_OpenRead: case OP_OpenWrite: assert(pOp->opcode==OP_OpenWrite || pOp->p5==0 || pOp->p5==OPFLAG_SEEKEQ); - /* - * Even if schema has changed, pointer can come from - * OP_SIDtoPtr opcode, which converts space id to pointer - * during runtime. - */ - if (box_schema_version() != p->schema_ver && - (pOp->p5 & OPFLAG_FRESH_PTR) == 0) { + if (box_schema_version() != p->schema_ver) { p->expired = 1; rc = SQLITE_ERROR; sqlite3VdbeError(p, "schema version has changed: " \ @@ -3208,9 +3176,7 @@ case OP_OpenWrite: goto abort_due_to_error; } p2 = pOp->p2; - pIn3 = &aMem[pOp->p3]; - assert(pIn3->flags & MEM_Ptr); - struct space *space = ((struct space *) pIn3->u.p); + struct space *space = pOp->p4.space; assert(space != NULL); struct index *index = space_index(space, p2); assert(index != NULL); @@ -4461,26 +4427,6 @@ case OP_SDelete: { break; } -/* Opcode: SIDtoPtr P1 P2 * * * - * Synopsis: space id = P1, space[out] = r[P2] - * - * This opcode makes look up by space id and save found space - * into register, specified by the content of register P2. - * Such trick is needed during DLL routine, since schema may - * change and pointers become expired. - */ -case OP_SIDtoPtr: { - assert(pOp->p1 > 0); - assert(pOp->p2 >= 0); - - pIn2 = out2Prerelease(p, pOp); - struct space *space = space_by_id(pOp->p1); - assert(space != NULL); - pIn2->u.p = (void *) space; - pIn2->flags = MEM_Ptr; - break; -} - /* Opcode: IdxDelete P1 P2 P3 * * * Synopsis: key=r[P2@P3] * -- 2.7.4