From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 36A9C2DAB5 for ; Tue, 30 Oct 2018 16:03:49 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id I6vOY5XDsydu for ; Tue, 30 Oct 2018 16:03:49 -0400 (EDT) Received: from smtp5.mail.ru (smtp5.mail.ru [94.100.179.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E8F222DAAF for ; Tue, 30 Oct 2018 16:03:48 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v8 1/3] box: factor fiber_gc out of txn_commit References: From: Vladislav Shpilevoy Message-ID: <4a7a178a-7632-4f1a-5b94-67ef886c784d@tarantool.org> Date: Tue, 30 Oct 2018 23:03:45 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: "n.pettik" , tarantool-patches@freelists.org Cc: Imeev Mergen Thanks for the review! On 30/10/2018 17:30, n.pettik wrote: > > >> On 29 Oct 2018, at 20:33, imeevma@tarantool.org wrote: >> >> Now txn_commit is judge, jury and executioner. It both >> commits or rollbacks data, and collects it calling fiber_gc, >> which destroys the region. > > Nit: both commits and rollbacks. Fixed. > >> >> But SQL wants to use some transactional data after commit. It is >> autogenerated identifiers - a list of sequence values generated >> for autoincrement columns and explicit sequence:next() calls. >> >> It is possible to store the list on malloced mem inside Vdbe, but >> it complicates deallocation. > > What is the problem with deallocation? AFAIU it is enough to > simply iterate over the list and release each element - not big deal. > > If you want to use region, mb it is worth to store separate region > specially for VDBE? We already have it in parser, so what prevents > us for adding the same thing to VDBE? I guess we can store many > things there, not only list of ids. I understand that parser in its turn > has nothing in common (at least it should, except for analyze machinery) > with transaction routines, so separate region is likely to be more > reasonable for parser, but anyway... > I've decided to say more details. Parser never yields. This is why we can waste here any resources, rack and ruin everything, but at the end of parsing it should be returned back. Vdbe, on the contrary, yields. So it holds some system resources while other fibers can not use them. If we added a special region to Vdbe, it would steal slabs from the thread's slab cache, while other fibers may want to use it. Hence, when we use one region for all transactional data, including language specific, allocations are much less fragmented over different slabs. Is this explanation decent? Also, I do not agree, that 'deallocation is just iteration and it is ok'. It is O(n) iteration and freeing of heap objects. If a one inserted 10k rows with autogenerated ids, it would waste 10k heap fragments, 10k calls of malloc/free - in my opinion it is an abysmal overhead, but what is more, it can be avoided for free. Instead of 10k free() it boils down to deallocation of N slabs, where N = slab_size / (10k * 8); 8 - size of autogenerated it; slab size is at least 64Kb, so N = 64*1024/80000 < 1. It takes 1 deallocation vs 10k deallocations. So I think this refactoring is worth.