Tarantool development patches archive
 help / color / mirror / Atom feed
From: mechanik20051988 <mechanik20.05.1988@gmail.com>
To: v.shpilevoy@tarantool.org
Cc: mechanik20051988 <mechanik20.05.1988@gmail.com>,
	tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH] memtx: change small allocator behavior
Date: Wed, 23 Dec 2020 16:14:46 +0300	[thread overview]
Message-ID: <20201223131452.23424-1-mechanik20051988@gmail.com> (raw)
In-Reply-To: <cover.1608715671.git.mechanik20051988@gmail.com>

From: mechanik20051988 <mechanik20.05.1988@gmail.com>

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

  reply	other threads:[~2020-12-23 13:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23 13:14 [Tarantool-patches] [PATCH v2 0/3] " mechanik20051988
2020-12-23 13:14 ` mechanik20051988 [this message]
2020-12-24 15:13   ` [Tarantool-patches] [PATCH] memtx: " Vladislav Shpilevoy
     [not found]     ` <0076A088-8CBC-4238-9EEB-0C73EC516098@hxcore.ol>
2020-12-25  7:42       ` [Tarantool-patches] FW: " Evgeny Mekhanik
2020-12-28 12:10         ` Vladislav Shpilevoy
2020-12-23 13:14 ` [Tarantool-patches] [PATCH v2 1/3] small: implement new size class evaluation mechanik20051988
2020-12-24 15:13   ` Vladislav Shpilevoy
     [not found]     ` <A90D94B7-298A-4D1B-8134-6EE2ED45D615@hxcore.ol>
2020-12-25  7:48       ` [Tarantool-patches] FW: " Evgeny Mekhanik
2020-12-28 12:10     ` [Tarantool-patches] " Vladislav Shpilevoy
2020-12-23 13:14 ` [Tarantool-patches] [PATCH v2 2/3] test: add small allocator performance test mechanik20051988
2020-12-23 13:14 ` [Tarantool-patches] [PATCH v2 3/3] small: changed small allocator pool management mechanik20051988
2020-12-24 15:13   ` Vladislav Shpilevoy
     [not found]     ` <27E47303-4307-4713-BB8A-2427FED09DDE@hxcore.ol>
2020-12-25  7:52       ` [Tarantool-patches] FW: " Evgeny Mekhanik
2020-12-25  7:56       ` Evgeny Mekhanik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201223131452.23424-1-mechanik20051988@gmail.com \
    --to=mechanik20.05.1988@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] memtx: change small allocator behavior' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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