[tarantool-patches] [PATCH 1/1] core: introduce tt_static_sized_buf()

[tarantool-patches] [PATCH 1/1] core: introduce tt_static_sized_buf()

[Vladislav Shpilevoy]

Currently Tarantool has a global thread local array of 4 static
buffers, each 1028 bytes. It provides an API tt_static_buf()
allowing to return them one by one in a cycle. But sometimes it
is needed to obtain a bit bigger buffer. For example, to store
a UDP packet - ~1.5Kb. Technically these 4 static buffers are an
array of ~4Kb and nothing prevents from allowing to take several
contiguous buffers at once to store 'big' data.

This commit introduces function tt_static_sized_buf() allowing
to take up to 1028 * 4 bytes at once. It rotates the buffer index
in the same way as tt_static_buf() does.

A main motivation for this commit is a wish to use a single
global out-of-stack buffer to read UDP packets into it in the
SWIM library, and on the other hand do not pad out BSS section
with a new SWIM-special static buffer. Now SWIM uses stack for
this and in the incoming cryptography SWIM component it will need
more.
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/tt_static_sized_buf

 src/trivia/util.h        | 33 +++++++++++++++-
 test/unit/CMakeLists.txt |  3 ++
 test/unit/misc.c         | 83 ++++++++++++++++++++++++++++++++++++++++
 test/unit/misc.result    | 28 ++++++++++++++
 4 files changed, 145 insertions(+), 2 deletions(-)
 create mode 100644 test/unit/misc.c
 create mode 100644 test/unit/misc.result

diff --git a/src/trivia/util.h b/src/trivia/util.h
index d17badff3..351e92c7b 100644
--- a/src/trivia/util.h
+++ b/src/trivia/util.h
@@ -490,8 +490,11 @@ extern __thread char tt_static_buf_storage[TT_STATIC_BUF_COUNT]
 extern __thread int tt_static_buf_i;
 
 /**
- * Return a thread-local statically allocated temporary buffer of size
- * \a TT_STATIC_BUF_LEN
+ * Return a thread-local statically allocated temporary buffer of
+ * size @a TT_STATIC_BUF_LEN. The buffers are rotated on each call
+ * of this function in a cycle. It allows to use multiple static
+ * buffers sequentially to store several different temporary data
+ * parts.
  */
 static inline char *
 tt_static_buf(void)
@@ -500,6 +503,32 @@ tt_static_buf(void)
 	return tt_static_buf_storage[tt_static_buf_i];
 }
 
+/**
+ * Return a thread-local statically allocated temporary buffer
+ * with at least @a size continuous bytes. @a size should not
+ * be bigger than total size of all tt static buffers. This call
+ * also rotates static buffer index with respect to the allocated
+ * block count.
+ */
+static inline char *
+tt_static_sized_buf(int size)
+{
+	assert(size <= TT_STATIC_BUF_COUNT * TT_STATIC_BUF_LEN);
+	if (size <= TT_STATIC_BUF_LEN)
+		return tt_static_buf();
+	int block_count = size / TT_STATIC_BUF_LEN +
+			  ((size % TT_STATIC_BUF_LEN) != 0);
+	assert(block_count <= TT_STATIC_BUF_COUNT);
+	int block_first = (tt_static_buf_i + 1) % TT_STATIC_BUF_COUNT;
+	int block_last = block_first + block_count - 1;
+	if (block_last < TT_STATIC_BUF_COUNT) {
+		tt_static_buf_i = block_last;
+		return tt_static_buf_storage[block_first];
+	}
+	tt_static_buf_i = block_count - 1;
+	return tt_static_buf_storage[0];
+}
+
 /**
  * Return a null-terminated string for \a str of length \a len
  */
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index 368ceb649..9aa9364e6 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -216,6 +216,9 @@ target_link_libraries(sio.test unit core)
 add_executable(crypto.test crypto.c)
 target_link_libraries(crypto.test crypto unit)
 
+add_executable(misc.test misc.c)
+target_link_libraries(misc.test misc unit core)
+
 add_executable(swim.test swim.c swim_test_transport.c swim_test_ev.c
                swim_test_utils.c ${PROJECT_SOURCE_DIR}/src/version.c)
 target_link_libraries(swim.test unit swim)
diff --git a/test/unit/misc.c b/test/unit/misc.c
new file mode 100644
index 000000000..4d025c201
--- /dev/null
+++ b/test/unit/misc.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "trivia/util.h"
+#include "unit.h"
+
+static inline void
+check_static_sized_buf(int size, int first_pos, int last_pos)
+{
+	char *b = tt_static_sized_buf(size);
+	is(b, tt_static_buf_storage[first_pos], "allocated %d from %d",
+	   size, first_pos);
+	is(tt_static_buf_i, last_pos, "to %d", last_pos);
+}
+
+static void
+test_static_buffers(void)
+{
+	header();
+	plan(21);
+
+	check_static_sized_buf(10, 0, 0);
+	check_static_sized_buf(TT_STATIC_BUF_LEN, 1, 1);
+	check_static_sized_buf(TT_STATIC_BUF_LEN, 2, 2);
+	check_static_sized_buf(TT_STATIC_BUF_LEN + 1, 0, 1);
+
+	is(tt_static_buf(), tt_static_buf_storage[2],
+	   "not sized buf() takes 2");
+	check_static_sized_buf(10, 3, 3);
+
+	check_static_sized_buf(TT_STATIC_BUF_LEN * 2, 0, 1);
+	check_static_sized_buf(TT_STATIC_BUF_LEN * 2, 2, 3);
+	is(tt_static_buf(), tt_static_buf_storage[0],
+	   "sized_buf() and buf() use the same counter");
+	is(tt_static_buf_i, 0, "buf() occupied 0 after sized_buf() "\
+	   "had occupied 3");
+
+	check_static_sized_buf(TT_STATIC_BUF_LEN * TT_STATIC_BUF_COUNT, 0, 3);
+	check_static_sized_buf(0, 0, 0);
+
+	check_plan();
+	footer();
+}
+
+int
+main(void)
+{
+	header();
+	plan(1);
+
+	test_static_buffers();
+
+	int rc = check_plan();
+	footer();
+	return rc;
+}
diff --git a/test/unit/misc.result b/test/unit/misc.result
new file mode 100644
index 000000000..12341fdee
--- /dev/null
+++ b/test/unit/misc.result
@@ -0,0 +1,28 @@
+	*** main ***
+1..1
+	*** test_static_buffers ***
+    1..21
+    ok 1 - allocated 10 from 0
+    ok 2 - to 0
+    ok 3 - allocated 1028 from 1
+    ok 4 - to 1
+    ok 5 - allocated 1028 from 2
+    ok 6 - to 2
+    ok 7 - allocated 1029 from 0
+    ok 8 - to 1
+    ok 9 - not sized buf() takes 2
+    ok 10 - allocated 10 from 3
+    ok 11 - to 3
+    ok 12 - allocated 2056 from 0
+    ok 13 - to 1
+    ok 14 - allocated 2056 from 2
+    ok 15 - to 3
+    ok 16 - sized_buf() and buf() use the same counter
+    ok 17 - buf() occupied 0 after sized_buf() had occupied 3
+    ok 18 - allocated 4112 from 0
+    ok 19 - to 3
+    ok 20 - allocated 0 from 0
+    ok 21 - to 0
+ok 1 - subtests
+	*** test_static_buffers: done ***
+	*** main: done ***
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 1/1] core: introduce tt_static_sized_buf()

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/27 15:39]:

To solve your particular problem, you could simply increase the
static buffer size. This is a one-line patch. I suggest you do it
in scope of swim and be done with it.

Here is why:

Essentially, you introduce a new allocator. A very useful one,
indeed, but the protocol should follow one of an allocator, e.g.
provide alloc, realloc, and a (no-op) free. The allocator itself
should be moved to small/. You should check boundary conditions -
e.g. attempt to allocate more memory than the maximal size of the
static array - and return 0. tt_static_buf() then can be replaced
with static_alloc(desired size).

If you wish to do all of the above in a separate patch - you're
more than welcome.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] Re: [PATCH 1/1] core: introduce tt_static_sized_buf()

[Vladislav Shpilevoy]

On 28/04/2019 00:18, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/27 15:39]:
> 
> To solve your particular problem, you could simply increase the
> static buffer size. This is a one-line patch. I suggest you do it
> in scope of swim and be done with it.
> 
> Here is why:
> 
> Essentially, you introduce a new allocator. A very useful one,
> indeed, but the protocol should follow one of an allocator, e.g.
> provide alloc, realloc, and a (no-op) free. The allocator itself
> should be moved to small/. You should check boundary conditions -
> e.g. attempt to allocate more memory than the maximal size of the
> static array - and return 0. tt_static_buf() then can be replaced
> with static_alloc(desired size).
> 
> If you wish to do all of the above in a separate patch - you're
> more than welcome.

I did, see the new mail thread. But I did not realloc() nor free().
Realloc() just is not needed. Free() would cause too huge changes
in the code, because every single tt_sprintf and tt_static_buf would
require free() - it would be tremendous diff.

> 
> 
> -- 
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
>