From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4605E4696C0 for ; Wed, 4 Dec 2019 01:51:13 +0300 (MSK) References: <0a6806665bbc165dffc23967be57135a46bbad62.1574277369.git.korablev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <8bd6cb33-4a55-8f4c-60cf-bc54928f220e@tarantool.org> Date: Tue, 3 Dec 2019 23:51:11 +0100 MIME-Version: 1.0 In-Reply-To: <0a6806665bbc165dffc23967be57135a46bbad62.1574277369.git.korablev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 11/16] sql: introduce sql_stmt_sizeof() function List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , tarantool-patches@dev.tarantool.org 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. >