[tarantool-patches] [PATCH v2 4/4] sql: remove OP_LoadPtr

  • From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Tue, 10 Jul 2018 20:08:11 +0300

From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>

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


Other related posts: