[Tarantool-patches] [PATCH 0/3] Change small allocator behavior
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Dec 21 14:38:52 MSK 2020
I forward this email with some big lines truncated,
because it seems they broke our mail server.
-------- Forwarded Message --------
Subject: Re: [Tarantool-patches] [PATCH 0/3] Change small allocator behavior
Date: Sun, 20 Dec 2020 19:30:22 +0100
From: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
To: mechanik20051988 <mechanik20.05.1988 at gmail.com>, tarantool-patches at dev.tarantool.org
Hi!
Thanks for the patchset!
On 18.12.2020 14:35, mechanik20051988 via Tarantool-patches wrote:
> Branch: mechanik20051988/gh-5216-fix-strange-allocator-behavior
The branch fails CI:
https://gitlab.com/tarantool/tarantool/-/pipelines/231416323
> Issue: https://github.com/tarantool/tarantool/issues/5216
> Description: Now we allocate pools on the stage of allocator creation.
>
> *** BLURB HERE ***
You need to replace this placeholder with your description.
I opened the branch mechanik20051988/gh-5216-fix-strange-allocator-behavior,
and I see that you didn't send the commit on top of this branch. So
I paste it below along with a review.
See 5 comments below.
> Change small allocator behavior, add test for #5216 issue
1. Please, read https://github.com/tarantool/tarantool/wiki/Code-review-procedure
carefully. You need to use subsystem name prefix for the commit
title. Here I assume it should be 'memtx: <...>'.
> Add a check that the slab_alloc_factor is in the range
> (1.0, 2.0] and if it is not so changed it to 1.0001 or 2.0
> respectively
2. Please, at least in the commit messages try to use punctuation.
Put dot in the end of sentences.
Also I guess you mean 'changed' -> 'change'.
Besides, I don't understand why is there such a restriction. Why
slab_alloc_factor 3 is bad, for example? I don't see a word about
that in the comments, in the commit message, nor in the issue
description on github (where the factor range is not even mentioned).
In addition, I don't see how 5216 is related to the factor borders.
> Closes #5216
>
> diff --git a/test/engine/small_alloc_factor.lua b/test/engine/small_alloc_factor.lua
> new file mode 100644
> index 000000000..b1089650e
> --- /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',
3. This config has trailing whitespaces. Git diff highlights
them with red color. Please, remove them.
> + memtx_memory = 2*1024*1024*1024,
4. Do you really need so much memory?
> + 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..251a62bee
> --- /dev/null
> +++ b/test/engine/small_alloc_factor.result
> @@ -0,0 +1,73 @@
> +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)}
> +---
> +- [477, 'aaaaaaaaaaaaaa...
> +...
5. The line is so huge that my email client lags. Git diff
too. Please, don't print it. The same for the other long
tuples. You are not supposed to test the console here.
Also I can't build the branch:
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:224:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, small_table_title);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:224:19: note: treat the string as an argument to avoid this
fprintf(stderr, small_table_title);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:225:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_2);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:225:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_2);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:226:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, small_table_title_1);
^~~~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:226:19: note: treat the string as an argument to avoid this
fprintf(stderr, small_table_title_1);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:227:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_3);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:227:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_3);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:257:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_3);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:257:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_3);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:258:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, small_table_title_3);
^~~~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:258:19: note: treat the string as an argument to avoid this
fprintf(stderr, small_table_title_3);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:259:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_2);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:259:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_2);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:260:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, small_table_title_1);
^~~~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:260:19: note: treat the string as an argument to avoid this
fprintf(stderr, small_table_title_1);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:261:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_3);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:261:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_3);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:285:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_2);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:285:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_2);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:300:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, large_table_title);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:300:19: note: treat the string as an argument to avoid this
fprintf(stderr, large_table_title);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:301:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_2);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:301:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_2);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:302:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, small_table_title_1);
^~~~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:302:19: note: treat the string as an argument to avoid this
fprintf(stderr, small_table_title_1);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:303:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_3);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:303:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_3);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:324:19: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_2);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:324:19: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_2);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:347:20: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_1);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:347:20: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_1);
^
"%s",
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:349:20: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
fprintf(stderr, underline_title_2);
^~~~~~~~~~~~~~~~~
/Users/gerold/Work/Repositories/tarantool/src/lib/small/perf/small_alloc_perf.c:349:20: note: treat the string as an argument to avoid this
fprintf(stderr, underline_title_2);
More information about the Tarantool-patches
mailing list