From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (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 961FF4765E0 for ; Thu, 24 Dec 2020 19:57:32 +0300 (MSK) Date: Thu, 24 Dec 2020 16:57:31 +0000 From: Nikita Pettik Message-ID: <20201224165730.GC14559@tarantool.org> References: <848e5b2fe5d1b8da491759a07abc035142280638.1607638829.git.lvasiliev@tarantool.org> <20201211120913.GB12730@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH] sql: increase default cache size List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org On 11 Dec 18:29, Leonid Vasiliev wrote: IMHO before pushing performance aimed patches, we should carefully review them. Could you run some SQL benchmarks to verify there's no degradation on any case? Also, does this change affect non-SQL users? I mean does SQL tmp cache is lazy initialized or it allocates all 20mb right on start? For sure, 18mb is not a lot memory in 2020, but still it may turn out to be surprise for some users.. > On 11.12.2020 15:09, Nikita Pettik wrote: > > On 11 Dec 01:37, Leonid Vasiliev wrote: > > > Increase the maximum number of in-memory pages to use > > > for temporary tables. > > > (https://www.sqlite.org/compile.html#default_cache_size) > > > > > > Part of #5609 > > > --- > > > > > > Hi SQL team. It is a simplest part of #5609. > > > This patch increases the default SQL cache size by 10 times to 20 MB. > > > A similar experiment shows a 10% performance increase for some datasets. > > > (https://github.com/tarantool/tarantool/issues/5593#issuecomment-740044007). > > > Mons approved these changes. I haven't done any research to determine the > > > optimal value. So feel free to throw this patch away. > > > > Hi, why not inroduce separate handler for user to change this setting? > > Since SQLite is embedded database, almost all settings are set at > > compile time. In constrast, we can move it to the box. > > > > If I understood you correctly, the settings should be implemented when > working on # 5609 (this path doesn't close the task). This is the > simplest improvement to update the default. > > > > https://github.com/tarantool/tarantool/issues/5609 > > > https://github.com/tarantool/tarantool/tree/lvasiliev/gh-5609-increase-default-sql-sort-cache-size > > > > > > src/box/CMakeLists.txt | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt > > > index 19203f7..7372179 100644 > > > --- a/src/box/CMakeLists.txt > > > +++ b/src/box/CMakeLists.txt > > > @@ -218,6 +218,10 @@ if(CMAKE_BUILD_TYPE STREQUAL "Debug") > > > add_definitions(-DSQL_DEBUG=1) > > > endif() > > > add_definitions(-DSQL_TEST=1) > > > +# Set the maximum number of in-memory pages to use for temporary tables. > > > +# 20000 * 1024 = 20480000 bytes. > > > +# (https://www.sqlite.org/compile.html#default_cache_size) > > > +add_definitions(-DSQL_DEFAULT_CACHE_SIZE=-20000) > > > set(EXT_SRC_DIR ${CMAKE_SOURCE_DIR}/extra) > > > set(EXT_BIN_DIR ${CMAKE_BINARY_DIR}/extra) > > > -- > > > 2.7.4 > > >