From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BF8724765E0 for ; Wed, 23 Dec 2020 16:15:01 +0300 (MSK) Received: by mail-lf1-f47.google.com with SMTP id s26so39940084lfc.8 for ; Wed, 23 Dec 2020 05:15:01 -0800 (PST) From: mechanik20051988 Date: Wed, 23 Dec 2020 16:14:46 +0300 Message-Id: <20201223131452.23424-1-mechanik20051988@gmail.com> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] memtx: change small allocator behavior List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org Cc: mechanik20051988 , tarantool-patches@dev.tarantool.org From: mechanik20051988 Branch: https://github.com/tarantool/tarantool/tree/mechanik20051988/gh-5216-fix-strange-allocator-behavior Pull request: https://github.com/tarantool/tarantool/pull/5503 Thank you for the previous review. Here some answers on you questions. 1. I add subsystem name prefix for the commit title 2. As you can see at this moment, small_alloc_create already has restrictions on alloc_factor (see small.c in master branch) They change it silently. if (alloc_factor > 2.0) alloc_factor = 2.0; /* * Correct the user-supplied alloc_factor to ensure that * it actually produces growing object sizes. */ if (alloc->step_pool_objsize_max * alloc_factor < alloc->step_pool_objsize_max + STEP_SIZE) { alloc_factor = (alloc->step_pool_objsize_max + STEP_SIZE + 0.5)/ alloc->step_pool_objsize_max; } I only moved them in memtx_engine.c with an explicit message. 3. Fix whitespaces 4. I took an example from issue to show that everything works now. 5. Fixed. 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 | 8 +++ src/lib/small | 2 +- test/engine/engine.cfg | 3 ++ test/engine/small_alloc_factor.lua | 11 ++++ test/engine/small_alloc_factor.result | 70 +++++++++++++++++++++++++ test/engine/small_alloc_factor.test.lua | 20 +++++++ 6 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 test/engine/small_alloc_factor.lua create mode 100644 test/engine/small_alloc_factor.result create mode 100644 test/engine/small_alloc_factor.test.lua diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c index 4f1fe3a3f..3ab292f2e 100644 --- a/src/box/memtx_engine.c +++ b/src/box/memtx_engine.c @@ -1116,6 +1116,14 @@ 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 then 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, diff --git a/src/lib/small b/src/lib/small index fcac155db..2053b49df 160000 --- a/src/lib/small +++ b/src/lib/small @@ -1 +1 @@ -Subproject commit fcac155dba18e97b10bc668d1a2fae01184adc13 +Subproject commit 2053b49dff3cb419245d243e96dbe981625be6d2 diff --git a/test/engine/engine.cfg b/test/engine/engine.cfg index f017a03ec..4a6a8bbbf 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"} + }, + "small_alloc_factor.test.lua" : { + "memtx": {"engine": "memtx"} } } diff --git a/test/engine/small_alloc_factor.lua b/test/engine/small_alloc_factor.lua new file mode 100644 index 000000000..a303a95d4 --- /dev/null +++ b/test/engine/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/small_alloc_factor.result b/test/engine/small_alloc_factor.result new file mode 100644 index 000000000..31d0f8ffd --- /dev/null +++ b/test/engine/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/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/small_alloc_factor.test.lua b/test/engine/small_alloc_factor.test.lua new file mode 100644 index 000000000..4edc4e5c2 --- /dev/null +++ b/test/engine/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/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