From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (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 8B7984696C3 for ; Tue, 17 Mar 2020 01:53:32 +0300 (MSK) References: <642b3f52ff8670b139e62012b46fb45750e777cf.1581940900.git.k.sosnin@tarantool.org> <20200316141630.GF14628@tarantool.org> From: Vladislav Shpilevoy Message-ID: <35c443bb-9416-d7a2-0eb2-99c299dd5abb@tarantool.org> Date: Mon, 16 Mar 2020 23:53:31 +0100 MIME-Version: 1.0 In-Reply-To: <20200316141630.GF14628@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , Chris Sosnin Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for your comments! On 16/03/2020 15:16, Nikita Pettik wrote: > On 17 Feb 15:12, Chris Sosnin wrote: >> As it is mentioned in implementation, it is important >> that _session_settings options are sorted by name, so >> there is no need in linear lookup and we can replace it >> with binary search. > > Is there any sufficient performance benefit except for code complication?:) > I really doubt that this part should be optimized for such rare in > terms of usage place. We didn't measure perf increase, but for such a simple change it did not seem necessary. Talking of rareness of settings change - this is arguable. Firstly, every new session will change settings, if a user's project uses some of them permanently. Secondly, some settings, such as default engine, or constraints related, or similar can be changed on per-request basis, in case user wants to make some requests not checking FK, and others should check them, inside one session, for instance. Similar necessity can arise for other settings. I think we can actually measure how faster lookup became for that case. > What is more, there's only dozen entries, so > binary search doesn't really make any sence. Idk why Kirill assigned 2.4.1 It is only dozen for now. This will change. We already have read-only session coming. > milestone, IMHO there are way far more important things to elaborate on. Honestly, I was thinking there is not much to elaborate. This is trivial bsearch.