Re: [Tarantool-patches] [PATCH 2/4] box: add binary search for _session_settings space

[Nikita Pettik <korablev@tarantool.org>]

On 16 Mar 23:53, Vladislav Shpilevoy wrote:
> 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.

I mean there can be no even performance benefit at all since
binary search on small data sets can be slower than sequential
access (there's linear search mode even in bps tree, but it is
compile time option).
 
> 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.

I mean work on more improtant features or/and severe bugs. Nevermind,
it is already done anyway. Personally I would reject this patch if
there's no any performance increase (otherwise, it simply complicates
code).