Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] sql: increase default cache size
@ 2020-12-10 22:37 Leonid Vasiliev
  2020-12-11 12:09 ` Nikita Pettik
  2020-12-11 14:12 ` Sergey Ostanevich
  0 siblings, 2 replies; 9+ messages in thread
From: Leonid Vasiliev @ 2020-12-10 22:37 UTC (permalink / raw)
  To: v.shpilevoy, imeevma, korablev, sergos; +Cc: tarantool-patches

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-10 22:37 [Tarantool-patches] [PATCH] sql: increase default cache size Leonid Vasiliev
@ 2020-12-11 12:09 ` Nikita Pettik
  2020-12-11 15:29   ` Leonid Vasiliev
  2020-12-11 14:12 ` Sergey Ostanevich
  1 sibling, 1 reply; 9+ messages in thread
From: Nikita Pettik @ 2020-12-11 12:09 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: tarantool-patches, v.shpilevoy

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.
 
> 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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-10 22:37 [Tarantool-patches] [PATCH] sql: increase default cache size Leonid Vasiliev
  2020-12-11 12:09 ` Nikita Pettik
@ 2020-12-11 14:12 ` Sergey Ostanevich
  2020-12-11 15:24   ` Leonid Vasiliev
  1 sibling, 1 reply; 9+ messages in thread
From: Sergey Ostanevich @ 2020-12-11 14:12 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: Vladislav Shpilevoy, tarantool-patches

Hi!

Thanks for the patch! Since the memory allocation is done on per-request
basis, there’s no impact on the memory consumption beyond these requests.
Still it gives a real boost, so LGTM.

Create follow-up ticket to provide a user-visible runtime option under
box.cfg

Sergos



> On 11 Dec 2020, at 01:37, Leonid Vasiliev <lvasiliev@tarantool.org> 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.
> 
> 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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-11 14:12 ` Sergey Ostanevich
@ 2020-12-11 15:24   ` Leonid Vasiliev
  0 siblings, 0 replies; 9+ messages in thread
From: Leonid Vasiliev @ 2020-12-11 15:24 UTC (permalink / raw)
  To: Sergey Ostanevich; +Cc: Vladislav Shpilevoy, tarantool-patches


Hi! Thank you for the review.

On 11.12.2020 17:12, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the patch! Since the memory allocation is done on per-request
> basis, there’s no impact on the memory consumption beyond these requests.
> Still it gives a real boost, so LGTM.
> 
> Create follow-up ticket to provide a user-visible runtime option under
> box.cfg
> 

The ticket already exists, the patch is part of this ticket (does not
close it).

> Sergos
> 
> 
> 
>> On 11 Dec 2020, at 01:37, Leonid Vasiliev <lvasiliev@tarantool.org> 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.
>>
>> 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
>>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-11 12:09 ` Nikita Pettik
@ 2020-12-11 15:29   ` Leonid Vasiliev
  2020-12-24 16:57     ` Nikita Pettik
  0 siblings, 1 reply; 9+ messages in thread
From: Leonid Vasiliev @ 2020-12-11 15:29 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy



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
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-11 15:29   ` Leonid Vasiliev
@ 2020-12-24 16:57     ` Nikita Pettik
  2020-12-28  8:54       ` Leonid Vasiliev
  0 siblings, 1 reply; 9+ messages in thread
From: Nikita Pettik @ 2020-12-24 16:57 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: tarantool-patches, v.shpilevoy

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
> > > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-24 16:57     ` Nikita Pettik
@ 2020-12-28  8:54       ` Leonid Vasiliev
  2020-12-28 12:13         ` Nikita Pettik
  0 siblings, 1 reply; 9+ messages in thread
From: Leonid Vasiliev @ 2020-12-28  8:54 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy

Hi!

On 24.12.2020 19:57, Nikita Pettik wrote:
> 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?

Can you tell me where we have benchmarks for SQL and how to run them?

> 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..

The memory allocation is done on per-request basis:
"The default page cache implemention does not allocate the full amount
of cache memory all at once. Cache memory is allocated in smaller chunks
on an as-needed basis."

>   
>> 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
>>>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-28  8:54       ` Leonid Vasiliev
@ 2020-12-28 12:13         ` Nikita Pettik
  2020-12-30 13:02           ` Leonid Vasiliev
  0 siblings, 1 reply; 9+ messages in thread
From: Nikita Pettik @ 2020-12-28 12:13 UTC (permalink / raw)
  To: Leonid Vasiliev; +Cc: tarantool-patches, v.shpilevoy

On 28 Dec 11:54, Leonid Vasiliev wrote:
> Hi!
> 
> On 24.12.2020 19:57, Nikita Pettik wrote:
> > 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?
> 
> Can you tell me where we have benchmarks for SQL and how to run them?

We have tpc-h/tpc-c bench at least as I know:
https://github.com/tarantool/tpcc
https://github.com/tarantool/tpch

Please contact QA team to get instructions how to lauch them.
 
> > 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..
> 
> The memory allocation is done on per-request basis:
> "The default page cache implemention does not allocate the full amount
> of cache memory all at once. Cache memory is allocated in smaller chunks
> on an as-needed basis."

Ok, fine, at least it won't affect users which don't use SQL sub-system.
So then we should only make sure that cache increase only improves
SQL performance on standard benches.

> > > 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
> > > > > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: increase default cache size
  2020-12-28 12:13         ` Nikita Pettik
@ 2020-12-30 13:02           ` Leonid Vasiliev
  0 siblings, 0 replies; 9+ messages in thread
From: Leonid Vasiliev @ 2020-12-30 13:02 UTC (permalink / raw)
  To: Nikita Pettik, okoshovetc; +Cc: tarantool-patches, v.shpilevoy

[-- Attachment #1: Type: text/plain, Size: 3860 bytes --]




Hi! 
>Понедельник, 28 декабря 2020, 15:13 +03:00 от Nikita Pettik <korablev@tarantool.org>:
> 
>On 28 Dec 11:54, Leonid Vasiliev wrote:
>> Hi!
>>
>> On 24.12.2020 19:57, Nikita Pettik wrote:
>> > 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?
>>
>> Can you tell me where we have benchmarks for SQL and how to run them?
>
>We have tpc-h/tpc-c bench at least as I know:
>https://github.com/tarantool/tpcc
>https://github.com/tarantool/tpch
>
>Please contact QA team to get instructions how to lauch them.
> 
 
As I understood after conversation with Timur, Oleg can help me.
So, I'll add him to the mailing list.
Oleg, can you help me with the benchmarks?
 
>> > 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..
>>
>> The memory allocation is done on per-request basis:
>> "The default page cache implemention does not allocate the full amount
>> of cache memory all at once. Cache memory is allocated in smaller chunks
>> on an as-needed basis."
>
>Ok, fine, at least it won't affect users which don't use SQL sub-system.
>So then we should only make sure that cache increase only improves
>SQL performance on standard benches.
>
>> > > 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
>> > > > > 
 
 
--
Leonid Vasiliev
 

[-- Attachment #2: Type: text/html, Size: 6000 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-30 13:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-10 22:37 [Tarantool-patches] [PATCH] sql: increase default cache size Leonid Vasiliev
2020-12-11 12:09 ` Nikita Pettik
2020-12-11 15:29   ` Leonid Vasiliev
2020-12-24 16:57     ` Nikita Pettik
2020-12-28  8:54       ` Leonid Vasiliev
2020-12-28 12:13         ` Nikita Pettik
2020-12-30 13:02           ` Leonid Vasiliev
2020-12-11 14:12 ` Sergey Ostanevich
2020-12-11 15:24   ` Leonid Vasiliev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox