From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 243C071828; Tue, 23 Mar 2021 12:39:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 243C071828 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1616492386; bh=1n4QO4T7q3NnETbY23noGrp3bX+p1ZZRMEIkjQRCZOA=; h=To:Cc:Date:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=DOW7hQeddKGc+0zL/E7alUarCMpQ7mzQXref1t+0gtUx6EplHwArEDh83ViDE4KEg vnCiFME9FQV8Ae8K/t/nBb8sO4Sv7SaU0y8/ItoIJn4YFIMfEJ86bV8MXzzm+IH/KZ rgOww6gUMcbGXefXEIfU7+ybFuMtHUrcCd6FrcBE= Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (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 970206AAB3 for ; Tue, 23 Mar 2021 12:35:17 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 970206AAB3 Received: by smtp34.i.mail.ru with esmtpa (envelope-from ) id 1lOdRU-0003Jx-L7; Tue, 23 Mar 2021 12:35:17 +0300 To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Tue, 23 Mar 2021 12:35:16 +0300 Message-Id: <47268e24824a721fb2b78a465109ea694785130d.1616491731.git.imeevma@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD95D6E7CC48CB1F5F1DDD90A25A8FA528D0BFD61B598B81272182A05F538085040E185EEA85F124E9DBDA5BFE804925192B53A75CFB3E0047905FC64E2907D2DE3 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7C42AF033AFE07300EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637802D3462438662818638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C686AF563A045C75E577633E676065BE5E9747417E16C01C6A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE77A825AB47F0FC8649FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3253F27DA5A70FAF3117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF5AA846FF95C4EB5FBA3038C0950A5D36C8A9BA7A39EFB766EC990983EF5C0329BA3038C0950A5D36D5E8D9A59859A8B6BDF432DE90D8338E76E601842F6C81A1F004C906525384307823802FF610243DF43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B3BBE47FD9DD3FB59A8DF7F3B2552694A57739F23D657EF2B13377AFFFEAFD26923F8577A6DFFEA7C054662F8F2CA352F93EC92FD9297F6715571747095F342E857739F23D657EF2BD5E8D9A59859A8B698B1B85E56A716AC089D37D7C0E48F6C5571747095F342E857739F23D657EF2B6825BDBE14D8E7024847893F9AA87235E5BFE6E7EFDEDCD789D4C264860C145E X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8183A4AFAF3EA6BDC44C234C8B12C006B7AD54267400E4E38EC1C952C2AE7C54D6B73477EDE26BDAA05B1881A6453793CE9C32612AADDFBE061C61BE10805914D3804EBA3D8E7E5B87ABF8C51168CD8EBDB63AF70AF8205D7DCDC48ACC2A39D04F89CDFB48F4795C241BDAD6C7F3747799A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D341ADA1A41A420E9B2B749634F037694D736818AF368869E43A294FC1FC3F2FB6B5A35BF8338BD6C681D7E09C32AA3244C0ADACA5A7C158749CBCFE2C7311C5F2BF94338140B71B8EEFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojWJZv20R+6Ug4qX5etBNzOw== X-Mailru-Sender: 5C3750E245F362008BC1685FEC6306ED6BB09FB291E59AF2BDA5BFE80492519249EBE2174CCC94795105BD0848736F9966FEC6BF5C9C28D97E07721503EA2E00ED97202A5A4E92BF7402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: [Tarantool-patches] [PATCH v4 10/53] sql: introduce mem_destroy() X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: imeevma@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This patch introduces mem_destroy(). This function should be used to free and destroing all objects owned by MEM, if necessary. Part of #5818 --- src/box/sql/func.c | 6 +- src/box/sql/mem.c | 171 +++++++++++++----------------------------- src/box/sql/mem.h | 7 +- src/box/sql/vdbe.c | 6 +- src/box/sql/vdbeInt.h | 2 +- src/box/sql/vdbeapi.c | 4 +- src/box/sql/vdbemem.c | 2 +- 7 files changed, 66 insertions(+), 132 deletions(-) diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 5e4eff931..e600a9800 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -301,7 +301,7 @@ port_lua_get_vdbemem(struct port *base, uint32_t *size) return (struct sql_value *)val; error: for (int i = 0; i < argc; i++) - sqlVdbeMemRelease(&val[i]); + mem_destroy(&val[i]); region_truncate(region, region_svp); return NULL; } @@ -373,7 +373,7 @@ port_c_get_vdbemem(struct port *base, uint32_t *size) return (struct sql_value *) val; error: for (int i = 0; i < port->size; i++) - sqlVdbeMemRelease(&val[i]); + mem_destroy(&val[i]); region_truncate(region, region_svp); return NULL; } @@ -2102,7 +2102,7 @@ minMaxFinalize(sql_context * context) if (pRes->flags) { sql_result_value(context, pRes); } - sqlVdbeMemRelease(pRes); + mem_destroy(pRes); } } diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 242360a66..ec6aaab64 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -89,6 +89,38 @@ mem_create(struct Mem *mem) #endif } +static inline void +mem_clear(struct Mem *mem) +{ + if ((mem->flags & (MEM_Agg | MEM_Dyn | MEM_Frame)) != 0) { + if ((mem->flags & MEM_Agg) != 0) + sql_vdbemem_finalize(mem, mem->u.func); + assert((mem->flags & MEM_Agg) == 0); + if ((mem->flags & MEM_Dyn) != 0) { + assert(mem->xDel != SQL_DYNAMIC && mem->xDel != NULL); + mem->xDel((void *)mem->z); + } else if ((mem->flags & MEM_Frame) != 0) { + struct VdbeFrame *frame = mem->u.pFrame; + frame->pParent = frame->v->pDelFrame; + frame->v->pDelFrame = frame; + } + } + mem->flags = MEM_Null; + mem->field_type = field_type_MAX; +} + +void +mem_destroy(struct Mem *mem) +{ + mem_clear(mem); + if (mem->szMalloc > 0) + sqlDbFree(mem->db, mem->zMalloc); + mem->n = 0; + mem->z = NULL; + mem->szMalloc = 0; + mem->zMalloc = NULL; +} + static inline bool mem_has_msgpack_subtype(struct Mem *mem) { @@ -200,56 +232,6 @@ vdbeMemAddTerminator(Mem * pMem) return 0; } -/* - * If the memory cell contains a value that must be freed by - * invoking the external callback in Mem.xDel, then this routine - * will free that value. It also sets Mem.flags to MEM_Null. - * - * This is a helper routine for sqlVdbeMemSetNull() and - * for sqlVdbeMemRelease(). Use those other routines as the - * entry point for releasing Mem resources. - */ -static SQL_NOINLINE void -vdbeMemClearExternAndSetNull(Mem * p) -{ - assert(VdbeMemDynamic(p)); - if (p->flags & MEM_Agg) { - sql_vdbemem_finalize(p, p->u.func); - assert((p->flags & MEM_Agg) == 0); - testcase(p->flags & MEM_Dyn); - } - if (p->flags & MEM_Dyn) { - assert(p->xDel != SQL_DYNAMIC && p->xDel != 0); - p->xDel((void *)p->z); - } else if (p->flags & MEM_Frame) { - VdbeFrame *pFrame = p->u.pFrame; - pFrame->pParent = pFrame->v->pDelFrame; - pFrame->v->pDelFrame = pFrame; - } - p->flags = MEM_Null; -} - -/* - * Release memory held by the Mem p, both external memory cleared - * by p->xDel and memory in p->zMalloc. - * - * This is a helper routine invoked by sqlVdbeMemRelease() in - * the unusual case where there really is memory in p that needs - * to be freed. - */ -static SQL_NOINLINE void -vdbeMemClear(Mem * p) -{ - if (VdbeMemDynamic(p)) { - vdbeMemClearExternAndSetNull(p); - } - if (p->szMalloc) { - sqlDbFree(p->db, p->zMalloc); - p->szMalloc = 0; - } - p->z = 0; -} - /* * Make an shallow copy of pFrom into pTo. Prior contents of * pTo are freed. The pFrom->z field is not duplicated. If @@ -259,7 +241,7 @@ vdbeMemClear(Mem * p) static SQL_NOINLINE void vdbeClrCopy(Mem * pTo, const Mem * pFrom, int eType) { - vdbeMemClearExternAndSetNull(pTo); + mem_clear(pTo); assert(!VdbeMemDynamic(pTo)); sqlVdbeMemShallowCopy(pTo, pFrom, eType); } @@ -1148,7 +1130,7 @@ sqlVdbeMemGrow(Mem * pMem, int n, int bPreserve) pMem->zMalloc = sqlDbMallocRaw(pMem->db, n); } if (pMem->zMalloc == 0) { - sqlVdbeMemSetNull(pMem); + mem_clear(pMem); pMem->z = 0; pMem->szMalloc = 0; return -1; @@ -1200,7 +1182,7 @@ sqlVdbeMemClearAndResize(Mem * pMem, int szNew) void mem_set_bool(struct Mem *mem, bool value) { - sqlVdbeMemSetNull(mem); + mem_clear(mem); mem->u.b = value; mem->flags = MEM_Bool; mem->field_type = FIELD_TYPE_BOOLEAN; @@ -1209,7 +1191,7 @@ mem_set_bool(struct Mem *mem, bool value) void mem_set_ptr(struct Mem *mem, void *ptr) { - sqlVdbeMemRelease(mem); + mem_destroy(mem); mem->flags = MEM_Ptr; mem->u.p = ptr; } @@ -1217,8 +1199,7 @@ mem_set_ptr(struct Mem *mem, void *ptr) void mem_set_i64(struct Mem *mem, int64_t value) { - if (VdbeMemDynamic(mem)) - sqlVdbeMemSetNull(mem); + mem_clear(mem); mem->u.i = value; int flag = value < 0 ? MEM_Int : MEM_UInt; MemSetTypeFlag(mem, flag); @@ -1228,8 +1209,7 @@ mem_set_i64(struct Mem *mem, int64_t value) void mem_set_u64(struct Mem *mem, uint64_t value) { - if (VdbeMemDynamic(mem)) - sqlVdbeMemSetNull(mem); + mem_clear(mem); mem->u.u = value; MemSetTypeFlag(mem, MEM_UInt); mem->field_type = FIELD_TYPE_UNSIGNED; @@ -1238,8 +1218,7 @@ mem_set_u64(struct Mem *mem, uint64_t value) void mem_set_int(struct Mem *mem, int64_t value, bool is_neg) { - if (VdbeMemDynamic(mem)) - sqlVdbeMemSetNull(mem); + mem_clear(mem); if (is_neg) { assert(value < 0); mem->u.i = value; @@ -1254,7 +1233,7 @@ mem_set_int(struct Mem *mem, int64_t value, bool is_neg) void mem_set_double(struct Mem *mem, double value) { - sqlVdbeMemSetNull(mem); + mem_clear(mem); if (sqlIsNaN(value)) return; mem->u.r = value; @@ -1291,7 +1270,7 @@ sqlVdbeMemSetStr(Mem * pMem, /* Memory cell to set to string value */ /* If z is a NULL pointer, set pMem to contain an SQL NULL. */ if (!z) { - sqlVdbeMemSetNull(pMem); + mem_clear(pMem); return 0; } @@ -1331,11 +1310,11 @@ sqlVdbeMemSetStr(Mem * pMem, /* Memory cell to set to string value */ } memcpy(pMem->z, z, nAlloc); } else if (xDel == SQL_DYNAMIC) { - sqlVdbeMemRelease(pMem); + mem_destroy(pMem); pMem->zMalloc = pMem->z = (char *)z; pMem->szMalloc = sqlDbMallocSize(pMem->db, pMem->zMalloc); } else { - sqlVdbeMemRelease(pMem); + mem_destroy(pMem); pMem->z = (char *)z; pMem->xDel = xDel; flags |= ((xDel == SQL_STATIC) ? MEM_Static : MEM_Dyn); @@ -1363,21 +1342,17 @@ sqlVdbeMemSetStr(Mem * pMem, /* Memory cell to set to string value */ * * This routine calls the Mem.xDel destructor to dispose of values that * require the destructor. But it preserves the Mem.zMalloc memory allocation. - * To free all resources, use sqlVdbeMemRelease(), which both calls this + * To free all resources, use mem_destroy(), which both calls this * routine to invoke the destructor and deallocates Mem.zMalloc. * * Use this routine to reset the Mem prior to insert a new value. * - * Use sqlVdbeMemRelease() to complete erase the Mem prior to abandoning it. + * Use mem_destroy() to complete erase the Mem prior to abandoning it. */ void sqlVdbeMemSetNull(Mem * pMem) { - if (VdbeMemDynamic(pMem)) { - vdbeMemClearExternAndSetNull(pMem); - } else { - pMem->flags = MEM_Null; - } + mem_clear(pMem); } /* @@ -1387,7 +1362,7 @@ sqlVdbeMemSetNull(Mem * pMem) void sqlVdbeMemSetZeroBlob(Mem * pMem, int n) { - sqlVdbeMemRelease(pMem); + mem_destroy(pMem); pMem->flags = MEM_Blob | MEM_Zero; pMem->n = 0; if (n < 0) @@ -1424,7 +1399,7 @@ sqlValueFree(sql_value * v) { if (!v) return; - sqlVdbeMemRelease((Mem *) v); + mem_destroy((Mem *) v); sqlDbFree(((Mem *) v)->db, v); } @@ -1447,34 +1422,10 @@ releaseMemArray(Mem * p, int N) { if (p && N) { Mem *pEnd = &p[N]; - sql *db = p->db; do { assert((&p[1]) == pEnd || p[0].db == p[1].db); assert(sqlVdbeCheckMemInvariants(p)); - - /* This block is really an inlined version of sqlVdbeMemRelease() - * that takes advantage of the fact that the memory cell value is - * being set to NULL after releasing any dynamic resources. - * - * The justification for duplicating code is that according to - * callgrind, this causes a certain test case to hit the CPU 4.7 - * percent less (x86 linux, gcc version 4.1.2, -O6) than if - * sqlMemRelease() were called from here. With -O2, this jumps - * to 6.6 percent. The test case is inserting 1000 rows into a table - * with no indexes using a single prepared INSERT statement, bind() - * and reset(). Inserts are grouped into a transaction. - */ - testcase(p->flags & MEM_Agg); - testcase(p->flags & MEM_Dyn); - testcase(p->flags & MEM_Frame); - if (p-> - flags & (MEM_Agg | MEM_Dyn | MEM_Frame)) { - sqlVdbeMemRelease(p); - } else if (p->szMalloc) { - sqlDbFree(db, p->zMalloc); - p->szMalloc = 0; - } - + mem_destroy(p); p->flags = MEM_Undefined; } while ((++p) < pEnd); } @@ -1916,8 +1867,7 @@ sqlVdbeMemCopy(Mem * pTo, const Mem * pFrom) { int rc = 0; - if (VdbeMemDynamic(pTo)) - vdbeMemClearExternAndSetNull(pTo); + mem_clear(pTo); memcpy(pTo, pFrom, MEMCELLSIZE); pTo->flags &= ~MEM_Dyn; if (pTo->flags & (MEM_Str | MEM_Blob)) { @@ -1957,7 +1907,7 @@ sqlVdbeMemMove(Mem * pTo, Mem * pFrom) { assert(pFrom->db == 0 || pTo->db == 0 || pFrom->db == pTo->db); - sqlVdbeMemRelease(pTo); + mem_destroy(pTo); memcpy(pTo, pFrom, sizeof(Mem)); pFrom->flags = MEM_Null; pFrom->szMalloc = 0; @@ -1992,25 +1942,6 @@ sqlVdbeMemMakeWriteable(Mem * pMem) return 0; } -/* - * Release any memory resources held by the Mem. Both the memory that is - * free by Mem.xDel and the Mem.zMalloc allocation are freed. - * - * Use this routine prior to clean up prior to abandoning a Mem, or to - * reset a Mem back to its minimum memory utilization. - * - * Use sqlVdbeMemSetNull() to release just the Mem.xDel space - * prior to inserting new content into the Mem. - */ -void -sqlVdbeMemRelease(Mem * p) -{ - assert(sqlVdbeCheckMemInvariants(p)); - if (VdbeMemDynamic(p) || p->szMalloc) { - vdbeMemClear(p); - } -} - int sql_vdbemem_finalize(struct Mem *mem, struct func *func) { diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 9a3b275fc..5ac02c002 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -100,6 +100,12 @@ mem_str(const struct Mem *mem); void mem_create(struct Mem *mem); +/** + * Destroy MEM and set NULL. + */ +void +mem_destroy(struct Mem *mem); + /* One or more of the following flags are set to indicate the validOK * representations of the value stored in the Mem struct. * @@ -426,7 +432,6 @@ int sqlVdbeMemCopy(Mem *, const Mem *); void sqlVdbeMemShallowCopy(Mem *, const Mem *, int); void sqlVdbeMemMove(Mem *, Mem *); int sqlVdbeMemMakeWriteable(Mem *); -void sqlVdbeMemRelease(Mem * p); /** * Memory cell mem contains the context of an aggregate function. diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 5378bc92b..12712efb4 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2471,7 +2471,7 @@ case OP_MakeRecord: { * to be passed to Tarantool. Before that, make * sure previously allocated memory has gone. */ - sqlVdbeMemRelease(pOut); + mem_destroy(pOut); pOut->flags = MEM_Blob | MEM_Ephem; pOut->n = tuple_size; pOut->z = tuple; @@ -4425,7 +4425,7 @@ case OP_Program: { /* jump */ if (!pFrame) { goto no_mem; } - sqlVdbeMemRelease(pRt); + mem_destroy(pRt); pRt->flags = MEM_Frame; pRt->u.pFrame = pFrame; @@ -4718,7 +4718,7 @@ case OP_AggStep: { struct func_sql_builtin *func = (struct func_sql_builtin *)pCtx->func; func->call(pCtx, pCtx->argc, pCtx->argv); if (pCtx->is_aborted) { - sqlVdbeMemRelease(&t); + mem_destroy(&t); goto abort_due_to_error; } assert(t.flags==MEM_Null); diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index e619f6e59..338b8bc96 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -130,7 +130,7 @@ struct VdbeCursor { * is linked into the Vdbe.pDelFrame list. The contents of the Vdbe.pDelFrame * list is deleted when the VM is reset in VdbeHalt(). The reason for doing * this instead of deleting the VdbeFrame immediately is to avoid recursive - * calls to sqlVdbeMemRelease() when the memory cells belonging to the + * calls to mem_destroy() when the memory cells belonging to the * child frame are released. * * The currently executing frame is stored in Vdbe.pFrame. Vdbe.pFrame is diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 671338361..a195f8dfd 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -681,9 +681,7 @@ vdbeUnbind(Vdbe * p, int i) } i--; pVar = &p->aVar[i]; - sqlVdbeMemRelease(pVar); - pVar->flags = MEM_Null; - pVar->field_type = field_type_MAX; + mem_destroy(pVar); return 0; } diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index 263fe5b00..bb87bb902 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -579,7 +579,7 @@ sqlStat4ProbeFree(UnpackedRecord * pRec) int part_count = pRec->key_def->part_count; struct Mem *aMem = pRec->aMem; for (int i = 0; i < part_count; i++) - sqlVdbeMemRelease(&aMem[i]); + mem_destroy(&aMem[i]); sqlDbFree(aMem[0].db, pRec); } } -- 2.25.1