[Tarantool-patches] [PATCH v2 11/16] sql: introduce sql_stmt_sizeof() function
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Dec 4 01:51:11 MSK 2019
Thanks for the patch!
See 4 comments below.
On 20/11/2019 22:28, Nikita Pettik wrote:
> To implement memory quota of prepared statement cache, we have to
> estimate size of prepared statement. This function attempts at that.
>
> Part of #2592
> ---
> src/box/execute.h | 8 ++++++++
> src/box/sql/vdbeapi.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/src/box/execute.h b/src/box/execute.h
> index 6702a18cc..d5b4d8421 100644
> --- a/src/box/execute.h
> +++ b/src/box/execute.h
> @@ -116,6 +116,14 @@ extern const struct port_vtab port_sql_vtab;
> int
> sql_finalize(struct sql_stmt *stmt);
>
> +/**
> + * Calculate estimated size of memory occupied by VM.
> + * See sqlVdbeMakeReady() for details concerning allocated
> + * memory.
> + */
> +size_t
> +sql_stmt_sizeof(const struct sql_stmt *stmt);
> +
1. Lets call it 'sql_stmt_sizeof_estimated()'. To emphasize that
the size is not exact.
> /**
> * Prepare (compile into VDBE byte-code) statement.
> *
> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
> index da528a4dc..10135bb68 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -805,6 +805,59 @@ sql_stmt_schema_version(sql_stmt *stmt)
> return v->schema_ver;
> }
>
> +size_t
> +sql_stmt_sizeof(const sql_stmt *stmt)
2. You have 'struct' in the declaration, but not
here. I think struct should be added there too.
> +{
> + struct Vdbe *v = (struct Vdbe *) stmt;
> + size_t size = sizeof(*v);
> + /* Resulting set */
> + size += sizeof(struct Mem) * v->nResColumn * COLNAME_N;
3. Could we account column names too? aColName member.
> + /* Opcodes */
> + size += sizeof(struct VdbeOp) * v->nOp;
> + /* Memory cells */
> + size += sizeof(struct Mem) * v->nMem;
> + /* Bindings */
> + size += sizeof(struct Mem) * v->nVar;
> + /* Bindings included in the result set */
> + size += sizeof(uint32_t) * v->res_var_count;
> + /* Cursors */
> + size += sizeof(struct VdbeCursor *) * v->nCursor;
> +
> + for (int i = 0; i < v->nOp; ++i) {
> + /* Estimate size of p4 operand. */
> + if (v->aOp[i].p4type != P4_NOTUSED) {
4. You can reduce the indentation:
- Try to invert the check and do 'continue' when P4 is
unused;
- 'Case' should be aligned under 'switch' according to
the code style.
> + switch (v->aOp[i].p4type) {
> + case P4_DYNAMIC:
> + case P4_STATIC:
> + if (v->aOp[i].opcode == OP_Blob ||
> + v->aOp[i].opcode == OP_String)
> + size += v->aOp[i].p1;
> + else if (v->aOp[i].opcode == OP_String8)
> + size += strlen(v->aOp[i].p4.z);
> + break;
> + case P4_BOOL:
> + size += sizeof(v->aOp[i].p4.b);
> + break;
> + case P4_INT32:
> + size += sizeof(v->aOp[i].p4.i);
> + break;
> + case P4_UINT64:
> + case P4_INT64:
> + size += sizeof(*v->aOp[i].p4.pI64);
> + break;
> + case P4_REAL:
> + size += sizeof(*v->aOp[i].p4.pReal);
> + break;
> + default:
> + size += sizeof(v->aOp[i].p4.p);
> + break;
> + }
> + }
> + }
> + size += strlen(v->zSql);
> + return size;
> +}
> +
> /******************************* sql_bind_ **************************
> *
> * Routines used to attach values to wildcards in a compiled SQL statement.
>
More information about the Tarantool-patches
mailing list