From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 DBD24469719 for ; Fri, 25 Sep 2020 00:30:41 +0300 (MSK) References: <20200911215115.6622-1-roman.habibov@tarantool.org> <20200911215115.6622-5-roman.habibov@tarantool.org> <06ae460c-b078-cbbb-14b5-7970ba669d40@tarantool.org> <3B490721-31EA-4FC9-B38E-48F70BCC8C19@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Thu, 24 Sep 2020 23:30:39 +0200 MIME-Version: 1.0 In-Reply-To: <3B490721-31EA-4FC9-B38E-48F70BCC8C19@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v4 4/6] sql: use parser's region of "index" array List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org On 23.09.2020 16:25, Roman Khabibov wrote: > Hi Thanks for the review. > >> On Sep 17, 2020, at 17:53, Vladislav Shpilevoy wrote: >> >> Thanks for the patch! >> >> On 11.09.2020 23:51, Roman Khabibov wrote: >>> Allocate memory for the "index" array of ephemeral space on the >>> parser's region instead of a heap as it was before. Fixed a memory >>> leak that realloc() generated. >> >> You need to provide more info why it can't be stored on the heap, >> and why free() can't be simply called somewhere. > Actually, it can be stored on the heap. I thought that getting rid > of malloc/free is the main point of using region. Main point of the region - speed, and it is also harder to leave a memory leak, because the region is destroyed in the end of parsing entirely. > The region is also > well suited for this task, because space-> index is information that > is needed only for parsing and should be destroyed with parsed. I dropped > this patch and added free in the new patch. Well, you could also make a patch which makes the parser space use the region. No need to hurry making a patch to make the questions gone.