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 436E36EC5F; Tue, 13 Apr 2021 19:38:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 436E36EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618331908; bh=Givnm/2k0S6CUkVrlcw214YZonXabE4F+NHd8tjU/Tc=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=zoRFqiciVswddtNuzfUoeQD2vn3BU8P0/+xrb7tzX0qgk9zxSZJ7SCTPWVcvsqZH6 JOUqowN0thv4a/ImTRXk6tG6nSuCn0R6vLh/JvE2anxO5khJ4cJdxooPPFLer+M4sm y8xiGi4lTUv9cRSyCCQ7NVgEnWlUkFxVHUKcY9/g= 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 861CE6EC5F for ; Tue, 13 Apr 2021 19:38:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 861CE6EC5F Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lWM3W-00062A-Jg; Tue, 13 Apr 2021 19:38:27 +0300 Date: Tue, 13 Apr 2021 19:38:25 +0300 To: Vladislav Shpilevoy Message-ID: <20210413163825.GA174670@tarantool.org> References: <03a0defcf93a9c1ac1080d13f9178ea5ab33ae59.1617984948.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E74806859AC5FE18436AEED970E897805ADA4182A05F5380850408138F58FD5ED12D84AEBB1912C5791F57D3B60AF9548FEFE97A173E36FA8738B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE74C265300876DF183C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7CF4D16325FBE1EEDEA1F7E6F0F101C67CDEEF6D7F21E0D1D9295C2E9FA3191EE1B59CA4C82EFA658E574E7817865970FF11E125022ED9BD1F6B57BC7E64490618DEB871D839B73339E8FC8737B5C22494854413538E1713FCC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C045A75973B56231AD8941B15DA834481F9449624AB7ADAF372E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F83C798A30B85E16B57739F23D657EF2BB5C8C57E37DE458BEDA766A37F9254B7 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2AD77751E876CB595E8F7B195E1C978317BC4A2B4CD6854DDA31E5B19C74FC7CE X-C1DE0DAB: 0D63561A33F958A54B394F2DF416C23BF0DE56C6B9264ABA103CDA927135587FD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D349379E7F8541B6C9AA431AFA81961CE5F4A5BB120130EF96E27E6D9CC8FC8A0E76C832CE207496AB91D7E09C32AA3244CD2A9B47B1C3FCE037764987C014CCF237C0C08F7987826B9FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnA7/qPBUIXGq3M9cHJfCLQ== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822E0E42A631E425966F6BC295687CC091A83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 15/52] sql: rework mem_move() 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: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thank you for the review! My answer, diff and new patch below. On Sun, Apr 11, 2021 at 08:10:50PM +0200, Vladislav Shpilevoy wrote: > I appreciate the work you did here! > > On 09.04.2021 19:37, Mergen Imeev via Tarantool-patches wrote: > > This patch reworks mem_move(). This function moves all content of source > > MEM to destination MEM. Source mem is set to NULL. > > > > Part of #5818 > > --- > > src/box/sql/mem.c | 57 +++++++++---------------------------------- > > src/box/sql/mem.h | 8 ++++-- > > src/box/sql/vdbe.c | 23 +---------------- > > src/box/sql/vdbeapi.c | 2 +- > > 4 files changed, 19 insertions(+), 71 deletions(-) > > > > diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c > > index f75661e04..d56fe56c6 100644 > > --- a/src/box/sql/mem.c > > +++ b/src/box/sql/mem.c > > @@ -315,6 +315,17 @@ mem_copy_as_ephemeral(struct Mem *to, const struct Mem *from) > > return; > > } > > > > +int > > It can be 'void'. The function never fails. > Fixed. > > +mem_move(struct Mem *to, struct Mem *from) > > +{ > > + mem_destroy(to); > > + memcpy(to, from, sizeof(*to)); > > + from->flags = MEM_Null; > > + from->szMalloc = 0; > > + from->zMalloc = NULL; > > + return 0; > > +} Diff: diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index f583dc7c0..2991f902d 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -170,7 +170,7 @@ mem_copy_as_ephemeral(struct Mem *to, const struct Mem *from) return; } -int +void mem_move(struct Mem *to, struct Mem *from) { mem_destroy(to); @@ -178,7 +178,6 @@ mem_move(struct Mem *to, struct Mem *from) from->flags = MEM_Null; from->szMalloc = 0; from->zMalloc = NULL; - return 0; } static inline bool diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 496fef1e3..beb9826a1 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -319,7 +319,7 @@ mem_copy_as_ephemeral(struct Mem *to, const struct Mem *from); /** * Move all content of source MEM to destination MEM. Source MEM is set to NULL. */ -int +void mem_move(struct Mem *to, struct Mem *from); /** New patch: commit e54a891eebb0ae9af9cde630628a5cdb03e00698 Author: Mergen Imeev Date: Fri Mar 5 15:30:23 2021 +0300 sql: rework mem_move() This patch reworks mem_move(). This function moves all content of source MEM to destination MEM. Source mem is set to NULL. Part of #5818 diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index befedf04c..2991f902d 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -170,6 +170,16 @@ mem_copy_as_ephemeral(struct Mem *to, const struct Mem *from) return; } +void +mem_move(struct Mem *to, struct Mem *from) +{ + mem_destroy(to); + memcpy(to, from, sizeof(*to)); + from->flags = MEM_Null; + from->szMalloc = 0; + from->zMalloc = NULL; +} + static inline bool mem_has_msgpack_subtype(struct Mem *mem) { @@ -1849,52 +1859,6 @@ vdbe_mem_alloc_blob_region(struct Mem *vdbe_mem, uint32_t size) return 0; } -/* - * Transfer the contents of pFrom to pTo. Any existing value in pTo is - * freed. If pFrom contains ephemeral data, a copy is made. - * - * pFrom contains an SQL NULL when this routine returns. - */ -void -sqlVdbeMemMove(Mem * pTo, Mem * pFrom) -{ - assert(pFrom->db == 0 || pTo->db == 0 || pFrom->db == pTo->db); - - mem_destroy(pTo); - memcpy(pTo, pFrom, sizeof(Mem)); - pFrom->flags = MEM_Null; - pFrom->szMalloc = 0; -} - -/* - * Change pMem so that its MEM_Str or MEM_Blob value is stored in - * MEM.zMalloc, where it can be safely written. - * - * Return 0 on success or -1 if malloc fails. - */ -int -sqlVdbeMemMakeWriteable(Mem * pMem) -{ - if ((pMem->flags & (MEM_Str | MEM_Blob)) != 0) { - if (ExpandBlob(pMem)) - return -1; - if (pMem->szMalloc == 0 || pMem->z != pMem->zMalloc) { - if (sqlVdbeMemGrow(pMem, pMem->n + 2, 1)) { - return -1; - } - pMem->z[pMem->n] = 0; - pMem->z[pMem->n + 1] = 0; - pMem->flags |= MEM_Term; - } - } - pMem->flags &= ~MEM_Ephem; -#ifdef SQL_DEBUG - pMem->pScopyFrom = 0; -#endif - - return 0; -} - 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 e30e5466e..beb9826a1 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -316,6 +316,12 @@ mem_copy(struct Mem *to, const struct Mem *from); void mem_copy_as_ephemeral(struct Mem *to, const struct Mem *from); +/** + * Move all content of source MEM to destination MEM. Source MEM is set to NULL. + */ +void +mem_move(struct Mem *to, struct Mem *from); + /** * Simple type to str convertor. It is used to simplify * error reporting. @@ -563,8 +569,6 @@ mem_is_type_compatible(struct Mem *mem, enum field_type type); int vdbe_mem_alloc_blob_region(struct Mem *vdbe_mem, uint32_t size); -void sqlVdbeMemMove(Mem *, Mem *); -int sqlVdbeMemMakeWriteable(Mem *); /** * 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 50d9dd78c..b3b20a374 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -197,21 +197,6 @@ vdbeTakeBranch(int iSrcLine, u8 I, u8 M) } #endif -/* - * An ephemeral string value (signified by the MEM_Ephem flag) contains - * a pointer to a dynamically allocated string where some other entity - * is responsible for deallocating that string. Because the register - * does not control the string, it might be deleted without the register - * knowing it. - * - * This routine converts an ephemeral string into a dynamically allocated - * string that the register itself controls. In other words, it - * converts an MEM_Ephem string into a string with P.z==P.zMalloc. - */ -#define Deephemeralize(P) \ - if (((P)->flags&MEM_Ephem)!=0 \ - && sqlVdbeMemMakeWriteable(P)) { goto no_mem;} - /* Return true if the cursor was opened using the OP_OpenSorter opcode. */ #define isSorter(x) ((x)->eCurType==CURTYPE_SORTER) @@ -981,13 +966,7 @@ case OP_Move: { assert(pIn1<=&aMem[(p->nMem+1 - p->nCursor)]); assert(memIsValid(pIn1)); memAboutToChange(p, pOut); - sqlVdbeMemMove(pOut, pIn1); -#ifdef SQL_DEBUG - if (pOut->pScopyFrom>=&aMem[p1] && pOut->pScopyFrompScopyFrom += pOp->p2 - p1; - } -#endif - Deephemeralize(pOut); + mem_move(pOut, pIn1); REGISTER_TRACE(p, p2++, pOut); pIn1++; pOut++; diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 7951996ea..8e69e3c38 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -980,7 +980,7 @@ sqlTransferBindings(sql_stmt * pFromStmt, sql_stmt * pToStmt) assert(pTo->db == pFrom->db); assert(pTo->nVar == pFrom->nVar); for (i = 0; i < pFrom->nVar; i++) { - sqlVdbeMemMove(&pTo->aVar[i], &pFrom->aVar[i]); + mem_move(&pTo->aVar[i], &pFrom->aVar[i]); } return 0; }