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 C95166EC5D; Fri, 9 Apr 2021 20:36:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C95166EC5D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617989814; bh=YJBlAdWCnEfv1p6qpjg0/5HQYngADJW2PrSF52MOdaw=; 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=SDm2dfEC/G3XL5HUrLX1Hh5DvtJWBk91Xim6bsm0ZkJamQYCORGBzF73zU/z7rdUc dLt9Eu6k5STqYl+RAWODTIHoMStrq+uhyCYaZsWsKX3/gZHpBUzyegg094S9yYc/2R E/e56AIlxzMvLJo0Xi6gRcj7970lA8PFWlzumLug= 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 dev.tarantool.org (Postfix) with ESMTPS id 9379B6EC5D for ; Fri, 9 Apr 2021 20:36:53 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9379B6EC5D Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lUv3s-0007U1-Je; Fri, 09 Apr 2021 20:36:53 +0300 To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Fri, 9 Apr 2021 20:36:52 +0300 Message-Id: <11ded2818e5dc4b3a60553c57d841de22be5bbe6.1617984948.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: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E74806859AC5FE18436AEED970E897805ADA4182A05F538085040D46AB303DCF0A9BB11DF52FF64E4E362F4B0D3DFFBE84E1984B951C02DD5C496 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E30B1A6D2A18B481EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F10F1F3256FD32E28638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B220667D743E80E74B1A3BA4A6CA8A6112BE5E7F44427E1A0ED2E47CDBA5A96583C09775C1D3CA48CFED8438A78DFE0A9E117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE7A4F00A9E8C511CEB9FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735C6EABA9B74D0DA47B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8183A4AFAF3EA6BDC44C234C8B12C006B7A36819FEE602673C73B1164D705E5D35DF310C77696E8C0C2B1881A6453793CE9C32612AADDFBE061C61BE10805914D3804EBA3D8E7E5B87ABF8C51168CD8EBDB63AF70AF8205D7DCDC48ACC2A39D04F89CDFB48F4795C241BDAD6C7F3747799A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D8C933888226C8417683689DDA01CBCAA922FEAC3F19404FBB6FF0E6271ED493F66334B23C7D63AA1D7E09C32AA3244C07FE6A4F5D8A4E6C087444205BD06AF6A995755A1445935EFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyO2lHpuZu4QpQ2FmXX5JVQ== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822950E89BC0BF3AEEC6890B50061DBC0DA83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v5 11/52] 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 | 2 +- src/box/sql/mem.c | 175 +++++++++++++----------------------------- src/box/sql/mem.h | 5 +- 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, 64 insertions(+), 132 deletions(-) diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 99ce938d5..13d8dd32c 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1796,7 +1796,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 5135637d9..805dc7054 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -92,6 +92,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) { @@ -203,56 +235,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 @@ -262,7 +244,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); } @@ -1108,7 +1090,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; @@ -1160,7 +1142,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; @@ -1169,7 +1151,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; } @@ -1177,8 +1159,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); @@ -1188,8 +1169,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; @@ -1198,8 +1178,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; @@ -1214,7 +1193,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; @@ -1251,7 +1230,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; } @@ -1291,11 +1270,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); @@ -1323,21 +1302,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); } /* @@ -1347,7 +1322,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) @@ -1384,7 +1359,7 @@ sqlValueFree(sql_value * v) { if (!v) return; - sqlVdbeMemRelease((Mem *) v); + mem_destroy((Mem *) v); sqlDbFree(((Mem *) v)->db, v); } @@ -1407,34 +1382,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); } @@ -1876,8 +1827,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)) { @@ -1917,7 +1867,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; @@ -1952,25 +1902,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) { @@ -2517,7 +2448,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; } @@ -2589,7 +2520,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; } diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 2a3d1078e..82b3084fb 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -98,6 +98,10 @@ mem_str(const struct Mem *mem); void mem_create(struct Mem *mem); +/** Free all allocated memory in MEM and set MEM to 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. * @@ -407,7 +411,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 74bf7f903..7cc72dc38 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2500,7 +2500,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; @@ -4454,7 +4454,7 @@ case OP_Program: { /* jump */ if (!pFrame) { goto no_mem; } - sqlVdbeMemRelease(pRt); + mem_destroy(pRt); pRt->flags = MEM_Frame; pRt->u.pFrame = pFrame; @@ -4747,7 +4747,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 b48e45770..cbf32ccdf 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