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 164E87030C; Tue, 9 Feb 2021 12:51:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 164E87030C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612864280; bh=6NR2OucYgSrOJKj/mz2aaLEQ1CUHOanMcQz8nSwDo/I=; h=To:Cc:References:In-Reply-To:Date:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=zGxfr0Og7v/bA0Eh8nwzutTYeovA017+wckVZy0UwLM8JiF6/hPww6n+ZXazp/v3o lQBNpvmCOT8dWIXxBhxCD3MEYCvVWR48crIFiYuxR36Z7IyV+OQ40blMEbuak3HKrF zzYKYWt55Yx7Hor3R/RXnzXlqhzv7JSCtv/gP3IQ= Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 7B3D17030C for ; Tue, 9 Feb 2021 12:51:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7B3D17030C Received: by smtp52.i.mail.ru with esmtpa (envelope-from ) id 1l9Pfx-000226-Md; Tue, 09 Feb 2021 12:51:18 +0300 To: "imeevma" , "Sergey Ostanevich" Cc: References: In-Reply-To: Date: Tue, 9 Feb 2021 12:51:14 +0300 Message-ID: <048e01d6fec9$1b363fb0$51a2bf10$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQH3nDARfbN/F+LiiguWA82wgsoPPwIaw6ObARCzg1Cp9P3IMA== Content-Language: ru X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD953AC099BC0052A9CC9F923E808BA4A11AAD7C69F0036037C182A05F5380850408857818D7AEE7125B4608A5538D8676341DCB9CE246A33CE8BBE95D8E318BFC3 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7B2CE06D3E4B8AFEBEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B0052F7941B60F658638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FC6F04D9B5D60EBDED885D89CD434E00E11893EF82FA65F91B389733CBF5DBD5E913377AFFFEAFD269A417C69337E82CC2CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD2691661749BA6B97735CA5E332AC8D387027B076A6E789B0E97A8DF7F3B2552694A1E7802607F20496D49FD398EE364050F7B96B19DC4093321B2D370F7B14D4BC4B3661434B16C20AC78D18283394535A975ECD9A6C639B01BC09775C1D3CA48CF7795E77BD711CB3535872C767BF85DA22EF20D2F80756B5F40A5AABA2AD3711975ECD9A6C639B01B78DA827A17800CE7D699F3A2029486C7731C566533BA786A40A5AABA2AD371193C9F3DD0FB1AF5EBDFC194086D65061027F269C8F02392CD5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A58315FA466E2EF4DE4EA94424FA57DB012940B984315FC5F8D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3441661D6226BE8C313D240C5FB4BFF8B5D1B15086FE3ED3EB36AB7BC074991EF2297FD4E5C2983BAF1D7E09C32AA3244C87E055573C411C47AE0E443F3CB22B8ED9ADFF0C0BDB8D1FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXjljbxDCxMK2cIUM2ptUCSP X-Mailru-Sender: 6CA451E36783D721CBEA96CEA26D325DEBF5BEC27680AFF1A66F26799A905A0EB7CBEF92542CD7C82F97C478340294DCC77752E0C033A69E0F0C7111264B8915FF1320A92A5534336C18EFA0BB12DBB0 X-Mras: Ok Subject: [Tarantool-patches] FW: [PATCH v1 09/10] sql: refactor vdbeaux.c 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: Timur Safin via Tarantool-patches Reply-To: Timur Safin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" -----Original Message----- From: Timur Safin Sent: Tuesday, February 9, 2021 12:50 PM To: 'imeevma@tarantool.org' Subject: RE: [PATCH v1 09/10] sql: refactor vdbeaux.c There are similar notes about neg/pos vs signed/unsigned in function names as I've mentioned earlier. But also some extra comments, please see below... : From: imeevma@tarantool.org : Subject: [PATCH v1 09/10] sql: refactor vdbeaux.c : : --- : src/box/sql/vdbeaux.c | 266 +++++++++++++++++++----------------------- : 1 file changed, 122 insertions(+), 144 deletions(-) : : diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c : index 7b8a1e1d8..4ffb34a4e 100644 : --- a/src/box/sql/vdbeaux.c : +++ b/src/box/sql/vdbeaux.c : @@ -1382,13 +1376,25 @@ sqlVdbeList(Vdbe * p) : if (apSub[j] == pOp->p4.pProgram) : break; : } : - if (j == nSub && : - sqlVdbeMemGrow(pSub, nByte, : - nSub != 0) == 0) { : - apSub = (SubProgram **) pSub->z; : - apSub[nSub++] = pOp->p4.pProgram; : - pSub->flags |= MEM_Blob; : - pSub->n = nSub * sizeof(SubProgram *); : + if (j == nSub) { : + size_t svp = region_used(&fiber()->gc); : + struct SubProgram **buf = (SubProgram **) : + region_aligned_alloc(&fiber()->gc, : + nByte, : + alignof(struct : SubProgram)); : + if (buf == NULL) { : + diag_set(OutOfMemory, nByte, : + "region_aligned_alloc", : + "buf"); : + p->is_aborted = true; : + return -1; : + } : + if (nSub > 0) : + memcpy(buf, pSub->z, pSub->n); : + buf[nSub++] = pOp->p4.pProgram; : + mem_set_bin(pSub, (char *)buf, nByte, 0, : + false); : + region_truncate(&fiber()->gc, svp); : } : } : } This was quite unexpected. I'd rather expect that refactoring would reduce number of code used inside of functions, not make them verbose. Could you please explain me why this extra code would become necessary? (And why not wrap them elsewhere?) : @@ -1402,41 +1408,26 @@ sqlVdbeList(Vdbe * p) : mem_set_i64(pMem, pOp->p3); : pMem++; : : - if (sqlVdbeMemClearAndResize(pMem, 256)) { : - assert(p->db->mallocFailed); : - return -1; : - } : - pMem->flags = MEM_Str | MEM_Term; : - zP4 = displayP4(pOp, pMem->z, pMem->szMalloc); : - : - if (zP4 != pMem->z) { : - pMem->n = 0; : - sqlVdbeMemSetStr(pMem, zP4, -1, 1, 0); : - } else { : - assert(pMem->z != 0); : - pMem->n = sqlStrlen30(pMem->z); : - } : + size_t size = 256; : + char *tmp_buf = (char *) static_alloc(size); : + assert(tmp_buf != NULL); : + zP4 = displayP4(pOp, tmp_buf, size); : + mem_set_str(pMem, zP4, strlen(zP4), 0, true); : pMem++; Please, please, not introduce any newer usage of static_alloc, it's very fragile and might work only inside of controlled context of single module (i.e. swim). If used by multiple modules - results are unpredictable. Any other allocator is fine if used by multiple fibers, but not static_alloc. Regards, Timur