From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 16DE44765E0 for ; Mon, 28 Dec 2020 17:11:36 +0300 (MSK) Date: Mon, 28 Dec 2020 17:11:33 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201228141133.GA156585@hpalx> References: <20201225114520.18796-1-mechanik20051988@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201225114520.18796-1-mechanik20051988@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4] memtx: change small allocator behavior List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mechanik20051988 Cc: tarantool-patches@dev.tarantool.org Hi Evgeny, thanks for the patchset, as I see no new degradation found in gitlab-ci testing commit criteria pipeline [1], patchset LGTM. [1] - https://gitlab.com/tarantool/tarantool/-/pipelines/235049916 On Fri, Dec 25, 2020 at 02:45:19PM +0300, mechanik20051988 via Tarantool-patches wrote: > From: mechanik20051988 > > Branch: https://github.com/tarantool/tarantool/tree/mechanik20051988/gh-5216-fix-strange-allocator-behavior > > Cnanges in v4: > Add actual_alloc_factor parameter to small_alloc_create > > Previously, in small allocator, memory pools > were allocated at the request, which in the case > of a small slab_alloc_factor led to use > pools with incorrect sizes. This patch changed > small allocator behavior, now we allocate pools > on the stage of allocator creation. Also we use > special function to find appropriate pool, which > is faster, then previous version with rbtree. > This change fixes #5216. > > Also moved a check, that the slab_alloc_factor is in > the range (1.0, 2.0] from small allocator to memtx_engine. > If factor is not in range change it to 1.0001 or 2.0 respectively > > Closes #5216 > --- > src/box/memtx_engine.c | 12 +++- > src/box/tuple.c | 3 +- > src/lib/small | 2 +- > test/engine/engine.cfg | 3 + > test/engine/gh-5216-small_alloc_factor.lua | 11 +++ > test/engine/gh-5216-small_alloc_factor.result | 70 +++++++++++++++++++ > .../gh-5216-small_alloc_factor.test.lua | 20 ++++++ > 7 files changed, 118 insertions(+), 3 deletions(-) > create mode 100644 test/engine/gh-5216-small_alloc_factor.lua > create mode 100644 test/engine/gh-5216-small_alloc_factor.result > create mode 100644 test/engine/gh-5216-small_alloc_factor.test.lua > > diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c > index 4f1fe3a3f..023402888 100644 > --- a/src/box/memtx_engine.c > +++ b/src/box/memtx_engine.c > @@ -1116,13 +1116,23 @@ memtx_engine_new(const char *snap_dirname, bool force_recovery, > if (objsize_min < OBJSIZE_MIN) > objsize_min = OBJSIZE_MIN; > > + if (alloc_factor > 2) { > + say_error("Alloc factor must be less than or equal to 2.0. It will be reduced to 2.0"); > + alloc_factor = 2.0; > + } else if (alloc_factor <= 1.0) { > + say_error("Alloc factor must be greater than 1.0. It will be increased to 1.001"); > + alloc_factor = 1.001; > + } > + > /* Initialize tuple allocator. */ > quota_init(&memtx->quota, tuple_arena_max_size); > tuple_arena_create(&memtx->arena, &memtx->quota, tuple_arena_max_size, > SLAB_SIZE, dontdump, "memtx"); > slab_cache_create(&memtx->slab_cache, &memtx->arena); > + float actual_alloc_factor; > small_alloc_create(&memtx->alloc, &memtx->slab_cache, > - objsize_min, alloc_factor); > + objsize_min, alloc_factor, &actual_alloc_factor); > + say_info("Actual alloc factor calculated on the basis of desired alloc_factor = %f", actual_alloc_factor); > > /* Initialize index extent allocator. */ > slab_cache_create(&memtx->index_slab_cache, &memtx->arena); > diff --git a/src/box/tuple.c b/src/box/tuple.c > index e83377c3a..c2023e4e8 100644 > --- a/src/box/tuple.c > +++ b/src/box/tuple.c > @@ -313,8 +313,9 @@ tuple_init(field_name_hash_f hash) > /* Make sure this one stays around. */ > tuple_format_ref(tuple_format_runtime); > > + float actual_alloc_factor; > small_alloc_create(&runtime_alloc, &cord()->slabc, OBJSIZE_MIN, > - ALLOC_FACTOR); > + ALLOC_FACTOR, &actual_alloc_factor); > > mempool_create(&tuple_iterator_pool, &cord()->slabc, > sizeof(struct tuple_iterator)); > diff --git a/src/lib/small b/src/lib/small > index fcac155db..bdbe73cd3 160000 > --- a/src/lib/small > +++ b/src/lib/small > @@ -1 +1 @@ > -Subproject commit fcac155dba18e97b10bc668d1a2fae01184adc13 > +Subproject commit bdbe73cd35865160dd9f80bfc8bfa3817f4980aa > diff --git a/test/engine/engine.cfg b/test/engine/engine.cfg > index f017a03ec..3bfc44dcf 100644 > --- a/test/engine/engine.cfg > +++ b/test/engine/engine.cfg > @@ -8,6 +8,9 @@ > }, > "gh-4973-concurrent-alter-fails.test.lua": { > "memtx": {"engine": "memtx"} > + }, > + "gh-5216-small_alloc_factor.test.lua" : { > + "memtx": {"engine": "memtx"} > } > } > > diff --git a/test/engine/gh-5216-small_alloc_factor.lua b/test/engine/gh-5216-small_alloc_factor.lua > new file mode 100644 > index 000000000..a303a95d4 > --- /dev/null > +++ b/test/engine/gh-5216-small_alloc_factor.lua > @@ -0,0 +1,11 @@ > +#!/usr/bin/env tarantool > + > +require('console').listen(os.getenv('ADMIN')) > + > +box.cfg({ > + wal_mode = 'none', > + memtx_memory = 2*1024*1024*1024, > + listen = os.getenv("LISTEN"), > + memtx_min_tuple_size=32, > + slab_alloc_factor=1.03 > +}) > diff --git a/test/engine/gh-5216-small_alloc_factor.result b/test/engine/gh-5216-small_alloc_factor.result > new file mode 100644 > index 000000000..5cf69a8f3 > --- /dev/null > +++ b/test/engine/gh-5216-small_alloc_factor.result > @@ -0,0 +1,70 @@ > +env = require('test_run') > +--- > +... > +test_run = env.new() > +--- > +... > +test_run:cmd('create server test with script="engine/gh-5216-small_alloc_factor.lua"') > +--- > +- true > +... > +test_run:cmd('start server test with args="none"') > +--- > +- true > +... > +test_run:cmd('switch test') > +--- > +- true > +... > +s = box.schema.space.create('test') > +--- > +... > +_ = s:create_index('test') > +--- > +... > +function str(i) return string.rep('a', math.floor(256 * math.pow(1.03, i))) end > +--- > +... > +for i=1,276 do _ = s:replace{i+200, str(i)} end > +--- > +... > +for i=1,274 do _ = s:delete{i+200} end > +--- > +... > +collectgarbage('collect') > +--- > +- 0 > +... > +_ = s:replace{200+277, str(275)} > +--- > +... > +_ = s:delete{200+275} > +--- > +... > +collectgarbage('collect') > +--- > +- 0 > +... > +_ = s:delete{200+276} > +--- > +... > +collectgarbage('collect') > +--- > +- 0 > +... > +test_run:cmd('switch default') > +--- > +- true > +... > +test_run:cmd('stop server test') > +--- > +- true > +... > +test_run:cmd('cleanup server test') > +--- > +- true > +... > +test_run:cmd('delete server test') > +--- > +- true > +... > diff --git a/test/engine/gh-5216-small_alloc_factor.test.lua b/test/engine/gh-5216-small_alloc_factor.test.lua > new file mode 100644 > index 000000000..83e51d19c > --- /dev/null > +++ b/test/engine/gh-5216-small_alloc_factor.test.lua > @@ -0,0 +1,20 @@ > +env = require('test_run') > +test_run = env.new() > +test_run:cmd('create server test with script="engine/gh-5216-small_alloc_factor.lua"') > +test_run:cmd('start server test with args="none"') > +test_run:cmd('switch test') > +s = box.schema.space.create('test') > +_ = s:create_index('test') > +function str(i) return string.rep('a', math.floor(256 * math.pow(1.03, i))) end > +for i=1,276 do _ = s:replace{i+200, str(i)} end > +for i=1,274 do _ = s:delete{i+200} end > +collectgarbage('collect') > +_ = s:replace{200+277, str(275)} > +_ = s:delete{200+275} > +collectgarbage('collect') > +_ = s:delete{200+276} > +collectgarbage('collect') > +test_run:cmd('switch default') > +test_run:cmd('stop server test') > +test_run:cmd('cleanup server test') > +test_run:cmd('delete server test') > -- > 2.20.1 >