[tarantool-patches] [PATCH 1/1] small: introduce static allocator

[tarantool-patches] [PATCH 1/1] small: introduce static allocator

[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, or on the contrary -
many smaller buffers. For example, to store a UDP packet -
~1.5Kb, or a small array of integers, probably not fitting into
1028 bytes.

This patch introduces a cyclic allocator having static thread
local buffer with fixed size 4096 * 3 bytes, and provides an API
to slice it by any smaller sizes. When the buffer is done, it is
recycled.

This allocator will allow to drop tt_static buffers and lots of
other static thread local buffers scattered over Tarantool source
code.
---
Branch: https://github.com/tarantool/small/tree/gerold103/static-allocator

 CMakeLists.txt      |  6 ++--
 small/static.c      | 34 ++++++++++++++++++
 small/static.h      | 85 +++++++++++++++++++++++++++++++++++++++++++++
 test/CMakeLists.txt |  4 +++
 test/static.c       | 77 ++++++++++++++++++++++++++++++++++++++++
 test/static.result  | 17 +++++++++
 6 files changed, 221 insertions(+), 2 deletions(-)
 create mode 100644 small/static.c
 create mode 100644 small/static.h
 create mode 100644 test/static.c
 create mode 100644 test/static.result

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ad27423..2b7d0dc 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -32,7 +32,8 @@ set(lib_headers
     small/slab_arena.h
     small/slab_cache.h
     small/small.h
-    small/lsregion.h)
+    small/lsregion.h
+    small/static.h)
 
 set(lib_sources
     small/slab_cache.c
@@ -43,7 +44,8 @@ set(lib_sources
     small/matras.c
     small/ibuf.c
     small/obuf.c
-    small/lsregion.c)
+    small/lsregion.c
+    small/static.c)
 
 add_library(${PROJECT_NAME} STATIC ${lib_sources})
 
diff --git a/small/static.c b/small/static.c
new file mode 100644
index 0000000..1ab7e03
--- /dev/null
+++ b/small/static.c
@@ -0,0 +1,34 @@
+/*
+ * 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 "static.h"
+
+__thread char static_storage_buffer[SMALL_STATIC_SIZE];
+__thread int static_storage_pos = 0;
diff --git a/small/static.h b/small/static.h
new file mode 100644
index 0000000..c6c040e
--- /dev/null
+++ b/small/static.h
@@ -0,0 +1,85 @@
+#ifndef TARANTOOL_SMALL_STATIC_H_INCLUDED
+#define TARANTOOL_SMALL_STATIC_H_INCLUDED
+/*
+ * 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 <stddef.h>
+#include <string.h>
+
+enum {
+	/**
+	 * Two pages (8192) would be too small for arbitrary
+	 * cases, but plus even 1 byte would occupy the whole new
+	 * page anyway, so here it is 3 pages.
+	 */
+	SMALL_STATIC_SIZE = 4096 * 3,
+};
+
+/**
+ * Thread-local statically allocated temporary BSS resident
+ * cyclic buffer.
+ */
+extern __thread char static_storage_buffer[SMALL_STATIC_SIZE];
+/** Next free position in the buffer. */
+extern __thread int static_storage_pos;
+
+/**
+ * Return a pointer onto the static buffer with at least @a size
+ * continuous bytes. When @a size is bigger than the static buffer
+ * size, then NULL is returned always. This is due to the fact the
+ * buffer is stored in BSS section - it is not dynamic and can't
+ * be extended. If there is not enough space till the end of the
+ * buffer, then it is recycled.
+ */
+static inline char *
+static_reserve(size_t size)
+{
+	if (static_storage_pos + size > SMALL_STATIC_SIZE) {
+		if (size > SMALL_STATIC_SIZE)
+			return NULL;
+		static_storage_pos = 0;
+	}
+	return &static_storage_buffer[static_storage_pos];
+}
+
+/**
+ * Reserve and propagate buffer position so as next allocations
+ * would not get the same pointer until the buffer is recycled.
+ */
+static inline char *
+static_alloc(size_t size)
+{
+	char *res = static_reserve(size);
+	if (res != NULL)
+		static_storage_pos += size;
+	return res;
+}
+
+#endif /* TARANTOOL_SMALL_STATIC_H_INCLUDED */
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 9520446..84d59dc 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -51,6 +51,9 @@ target_link_libraries(quota.test pthread)
 add_executable(quota_lessor.test quota_lessor.c unit.c)
 target_link_libraries(quota_lessor.test pthread)
 
+add_executable(static.test static.c unit.c)
+target_link_libraries(static.test pthread small)
+
 include_directories("${PROJECT_SOURCE_DIR}")
 
 add_test(slab_cache ${CMAKE_CURRENT_BUILD_DIR}/slab_cache.test)
@@ -69,6 +72,7 @@ add_test(quota_lessor ${CMAKE_CURRENT_BUILD_DIR}/quota_lessor.test)
 add_test(rb ${CMAKE_CURRENT_BUILD_DIR}/rb.test)
 add_test(rb_aug ${CMAKE_CURRENT_BUILD_DIR}/rb_aug.test)
 add_test(rb_rand ${CMAKE_CURRENT_BUILD_DIR}/rb_rand.test)
+add_test(static ${CMAKE_CURRENT_BUILD_DIR}/static.test)
 
 if(DEFINED SMALL_EMBEDDED)
     return()
diff --git a/test/static.c b/test/static.c
new file mode 100644
index 0000000..2d2f65b
--- /dev/null
+++ b/test/static.c
@@ -0,0 +1,77 @@
+/*
+ * 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 "small/static.h"
+#include "unit.h"
+
+static inline void
+check_static_alloc(int size, int first_pos, int end_pos)
+{
+	char *b = static_alloc(size);
+	is(b, &static_storage_buffer[first_pos], "allocated %d from %d",
+	   size, first_pos);
+	is(static_storage_pos, end_pos, "to %d", end_pos);
+}
+
+int
+main(void)
+{
+	header();
+	plan(14);
+
+	check_static_alloc(10, 0, 10);
+	int offset = 10;
+	int size = SMALL_STATIC_SIZE / 2;
+	check_static_alloc(size, offset, offset + size);
+	offset += size;
+
+	size = SMALL_STATIC_SIZE / 3;
+	check_static_alloc(size, offset, offset + size);
+	offset += size;
+
+	size = SMALL_STATIC_SIZE - offset;
+	check_static_alloc(size, offset, offset + size);
+
+	size = 1;
+	offset = 0;
+	check_static_alloc(size, offset, offset + size);
+
+	is(static_alloc(SMALL_STATIC_SIZE), static_storage_buffer,
+	   "can allocate the entire buffer");
+	is(static_storage_pos, SMALL_STATIC_SIZE, "position is updated");
+
+	is(static_alloc(SMALL_STATIC_SIZE + 1), NULL,
+	   "can't allocate more - the memory is static and can't be extended");
+	is(static_storage_pos, SMALL_STATIC_SIZE, "position is not changed");
+
+	int rc = check_plan();
+	footer();
+	return rc;
+}
diff --git a/test/static.result b/test/static.result
new file mode 100644
index 0000000..b487d74
--- /dev/null
+++ b/test/static.result
@@ -0,0 +1,17 @@
+	*** main ***
+1..14
+ok 1 - allocated 10 from 0
+ok 2 - to 10
+ok 3 - allocated 6144 from 10
+ok 4 - to 6154
+ok 5 - allocated 4096 from 6154
+ok 6 - to 10250
+ok 7 - allocated 2038 from 10250
+ok 8 - to 12288
+ok 9 - allocated 1 from 0
+ok 10 - to 1
+ok 11 - can allocate the entire buffer
+ok 12 - position is updated
+ok 13 - can't allocate more - the memory is static and can't be extended
+ok 14 - position is not changed
+	*** main: done ***
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Vladislav Shpilevoy]

Just a kind poke, that I still need your review here.

On 28/04/2019 21:41, Vladislav Shpilevoy wrote:
> 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, or on the contrary -
> many smaller buffers. For example, to store a UDP packet -
> ~1.5Kb, or a small array of integers, probably not fitting into
> 1028 bytes.
> 
> This patch introduces a cyclic allocator having static thread
> local buffer with fixed size 4096 * 3 bytes, and provides an API
> to slice it by any smaller sizes. When the buffer is done, it is
> recycled.
> 
> This allocator will allow to drop tt_static buffers and lots of
> other static thread local buffers scattered over Tarantool source
> code.
> ---
> Branch: https://github.com/tarantool/small/tree/gerold103/static-allocator
> 
>  CMakeLists.txt      |  6 ++--
>  small/static.c      | 34 ++++++++++++++++++
>  small/static.h      | 85 +++++++++++++++++++++++++++++++++++++++++++++
>  test/CMakeLists.txt |  4 +++
>  test/static.c       | 77 ++++++++++++++++++++++++++++++++++++++++
>  test/static.result  | 17 +++++++++
>  6 files changed, 221 insertions(+), 2 deletions(-)
>  create mode 100644 small/static.c
>  create mode 100644 small/static.h
>  create mode 100644 test/static.c
>  create mode 100644 test/static.result
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index ad27423..2b7d0dc 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -32,7 +32,8 @@ set(lib_headers
>      small/slab_arena.h
>      small/slab_cache.h
>      small/small.h
> -    small/lsregion.h)
> +    small/lsregion.h
> +    small/static.h)
>  
>  set(lib_sources
>      small/slab_cache.c
> @@ -43,7 +44,8 @@ set(lib_sources
>      small/matras.c
>      small/ibuf.c
>      small/obuf.c
> -    small/lsregion.c)
> +    small/lsregion.c
> +    small/static.c)
>  
>  add_library(${PROJECT_NAME} STATIC ${lib_sources})
>  
> diff --git a/small/static.c b/small/static.c
> new file mode 100644
> index 0000000..1ab7e03
> --- /dev/null
> +++ b/small/static.c
> @@ -0,0 +1,34 @@
> +/*
> + * 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 "static.h"
> +
> +__thread char static_storage_buffer[SMALL_STATIC_SIZE];
> +__thread int static_storage_pos = 0;
> diff --git a/small/static.h b/small/static.h
> new file mode 100644
> index 0000000..c6c040e
> --- /dev/null
> +++ b/small/static.h
> @@ -0,0 +1,85 @@
> +#ifndef TARANTOOL_SMALL_STATIC_H_INCLUDED
> +#define TARANTOOL_SMALL_STATIC_H_INCLUDED
> +/*
> + * 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 <stddef.h>
> +#include <string.h>
> +
> +enum {
> +	/**
> +	 * Two pages (8192) would be too small for arbitrary
> +	 * cases, but plus even 1 byte would occupy the whole new
> +	 * page anyway, so here it is 3 pages.
> +	 */
> +	SMALL_STATIC_SIZE = 4096 * 3,
> +};
> +
> +/**
> + * Thread-local statically allocated temporary BSS resident
> + * cyclic buffer.
> + */
> +extern __thread char static_storage_buffer[SMALL_STATIC_SIZE];
> +/** Next free position in the buffer. */
> +extern __thread int static_storage_pos;
> +
> +/**
> + * Return a pointer onto the static buffer with at least @a size
> + * continuous bytes. When @a size is bigger than the static buffer
> + * size, then NULL is returned always. This is due to the fact the
> + * buffer is stored in BSS section - it is not dynamic and can't
> + * be extended. If there is not enough space till the end of the
> + * buffer, then it is recycled.
> + */
> +static inline char *
> +static_reserve(size_t size)
> +{
> +	if (static_storage_pos + size > SMALL_STATIC_SIZE) {
> +		if (size > SMALL_STATIC_SIZE)
> +			return NULL;
> +		static_storage_pos = 0;
> +	}
> +	return &static_storage_buffer[static_storage_pos];
> +}
> +
> +/**
> + * Reserve and propagate buffer position so as next allocations
> + * would not get the same pointer until the buffer is recycled.
> + */
> +static inline char *
> +static_alloc(size_t size)
> +{
> +	char *res = static_reserve(size);
> +	if (res != NULL)
> +		static_storage_pos += size;
> +	return res;
> +}
> +
> +#endif /* TARANTOOL_SMALL_STATIC_H_INCLUDED */
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 9520446..84d59dc 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -51,6 +51,9 @@ target_link_libraries(quota.test pthread)
>  add_executable(quota_lessor.test quota_lessor.c unit.c)
>  target_link_libraries(quota_lessor.test pthread)
>  
> +add_executable(static.test static.c unit.c)
> +target_link_libraries(static.test pthread small)
> +
>  include_directories("${PROJECT_SOURCE_DIR}")
>  
>  add_test(slab_cache ${CMAKE_CURRENT_BUILD_DIR}/slab_cache.test)
> @@ -69,6 +72,7 @@ add_test(quota_lessor ${CMAKE_CURRENT_BUILD_DIR}/quota_lessor.test)
>  add_test(rb ${CMAKE_CURRENT_BUILD_DIR}/rb.test)
>  add_test(rb_aug ${CMAKE_CURRENT_BUILD_DIR}/rb_aug.test)
>  add_test(rb_rand ${CMAKE_CURRENT_BUILD_DIR}/rb_rand.test)
> +add_test(static ${CMAKE_CURRENT_BUILD_DIR}/static.test)
>  
>  if(DEFINED SMALL_EMBEDDED)
>      return()
> diff --git a/test/static.c b/test/static.c
> new file mode 100644
> index 0000000..2d2f65b
> --- /dev/null
> +++ b/test/static.c
> @@ -0,0 +1,77 @@
> +/*
> + * 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 "small/static.h"
> +#include "unit.h"
> +
> +static inline void
> +check_static_alloc(int size, int first_pos, int end_pos)
> +{
> +	char *b = static_alloc(size);
> +	is(b, &static_storage_buffer[first_pos], "allocated %d from %d",
> +	   size, first_pos);
> +	is(static_storage_pos, end_pos, "to %d", end_pos);
> +}
> +
> +int
> +main(void)
> +{
> +	header();
> +	plan(14);
> +
> +	check_static_alloc(10, 0, 10);
> +	int offset = 10;
> +	int size = SMALL_STATIC_SIZE / 2;
> +	check_static_alloc(size, offset, offset + size);
> +	offset += size;
> +
> +	size = SMALL_STATIC_SIZE / 3;
> +	check_static_alloc(size, offset, offset + size);
> +	offset += size;
> +
> +	size = SMALL_STATIC_SIZE - offset;
> +	check_static_alloc(size, offset, offset + size);
> +
> +	size = 1;
> +	offset = 0;
> +	check_static_alloc(size, offset, offset + size);
> +
> +	is(static_alloc(SMALL_STATIC_SIZE), static_storage_buffer,
> +	   "can allocate the entire buffer");
> +	is(static_storage_pos, SMALL_STATIC_SIZE, "position is updated");
> +
> +	is(static_alloc(SMALL_STATIC_SIZE + 1), NULL,
> +	   "can't allocate more - the memory is static and can't be extended");
> +	is(static_storage_pos, SMALL_STATIC_SIZE, "position is not changed");
> +
> +	int rc = check_plan();
> +	footer();
> +	return rc;
> +}
> diff --git a/test/static.result b/test/static.result
> new file mode 100644
> index 0000000..b487d74
> --- /dev/null
> +++ b/test/static.result
> @@ -0,0 +1,17 @@
> +	*** main ***
> +1..14
> +ok 1 - allocated 10 from 0
> +ok 2 - to 10
> +ok 3 - allocated 6144 from 10
> +ok 4 - to 6154
> +ok 5 - allocated 4096 from 6154
> +ok 6 - to 10250
> +ok 7 - allocated 2038 from 10250
> +ok 8 - to 12288
> +ok 9 - allocated 1 from 0
> +ok 10 - to 1
> +ok 11 - can allocate the entire buffer
> +ok 12 - position is updated
> +ok 13 - can't allocate more - the memory is static and can't be extended
> +ok 14 - position is not changed
> +	*** main: done ***
> -- 
> 2.20.1 (Apple Git-117)
> 
> 

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/28 21:42]:

OK to push.

Shouldn't you align the address or provide an aligned alloc along
with the basic one?

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

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Vladislav Shpilevoy]

On 04/05/2019 00:26, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/28 21:42]:
> 
> OK to push.
> 
> Shouldn't you align the address or provide an aligned alloc along
> with the basic one?

I thought about a function like static_aligned_alloc just like we
have for region, but was not sure if we need it now. There are no
code uses old tt_static_buf for data needed alignment. I hoped we
could implement it on demand.

But if you think we should implement it now, I can do that.
Should I?

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

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/04 00:34]:
> > OK to push.
> > 
> > Shouldn't you align the address or provide an aligned alloc along
> > with the basic one?
> 
> I thought about a function like static_aligned_alloc just like we
> have for region, but was not sure if we need it now. There are no
> code uses old tt_static_buf for data needed alignment. I hoped we
> could implement it on demand.
> 
> But if you think we should implement it now, I can do that.
> Should I?

Previously tt_static_buf allocations were always aligned, since
the next alloc would always allocate at position 0 in the buffer.
Now you provide a general purpose alloc more or less, which can
start at any position in the buffer. I would simple ensure that
static_alloc() is intptr_t aligned. We can add an aligned alloc
later.


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

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Vladislav Shpilevoy]

On 04/05/2019 00:36, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/04 00:34]:
>>> OK to push.
>>>
>>> Shouldn't you align the address or provide an aligned alloc along
>>> with the basic one?
>>
>> I thought about a function like static_aligned_alloc just like we
>> have for region, but was not sure if we need it now. There are no
>> code uses old tt_static_buf for data needed alignment. I hoped we
>> could implement it on demand.
>>
>> But if you think we should implement it now, I can do that.
>> Should I?
> 
> Previously tt_static_buf allocations were always aligned, since
> the next alloc would always allocate at position 0 in the buffer.
> Now you provide a general purpose alloc more or less, which can
> start at any position in the buffer. I would simple ensure that
> static_alloc() is intptr_t aligned. We can add an aligned alloc
> later.
If we align reserve() or alloc() by default, we would break the
pattern, when we do big reserve of maximal size and then lots of
small allocs of exact sizes. Works when we do not know how many
bytes a big complex object will occupy.

We use that for mpstream with obuf, ibuf, region, and somewhere in
SQL for pure region without mpstream. All our allocators guarantee
now, that if you succeed at a big reserve(), you can consume this
memory in multiple allocs, without gaps.

Additionally it contradicts with region API (the only allocator,
providing aligned allocations) - it has separate reserve + alloc, and
aligned_reserve + aligned_alloc.

I would rather implement separately normal static_alloc/reserve +
static_aligned_alloc/aligned_reserve.

Are you okay with that?

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

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/04 09:38]:
> I would rather implement separately normal static_alloc/reserve +
> static_aligned_alloc/aligned_reserve.
> 
> Are you okay with that?

OK


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

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Vladislav Shpilevoy]

I've sent a new patch in V2 thread.

On 04/05/2019 09:50, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/04 09:38]:
>> I would rather implement separately normal static_alloc/reserve +
>> static_aligned_alloc/aligned_reserve.
>>
>> Are you okay with that?
> 
> OK
> 
> 
> -- 
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
> 

[tarantool-patches] Re: [PATCH 1/1] small: introduce static allocator

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/04 09:38]:
> >>>
> >>> Shouldn't you align the address or provide an aligned alloc along
> >>> with the basic one?
> >>
> >> I thought about a function like static_aligned_alloc just like we
> >> have for region, but was not sure if we need it now. There are no
> >> code uses old tt_static_buf for data needed alignment. I hoped we
> >> could implement it on demand.
> >>
> >> But if you think we should implement it now, I can do that.
> >> Should I?
> > 
> > Previously tt_static_buf allocations were always aligned, since
> > the next alloc would always allocate at position 0 in the buffer.
> > Now you provide a general purpose alloc more or less, which can
> > start at any position in the buffer. I would simple ensure that
> > static_alloc() is intptr_t aligned. We can add an aligned alloc
> > later.
> If we align reserve() or alloc() by default, we would break the
> pattern, when we do big reserve of maximal size and then lots of
> small allocs of exact sizes. Works when we do not know how many
> bytes a big complex object will occupy.
> 
> We use that for mpstream with obuf, ibuf, region, and somewhere in
> SQL for pure region without mpstream. All our allocators guarantee
> now, that if you succeed at a big reserve(), you can consume this
> memory in multiple allocs, without gaps.
> 
> Additionally it contradicts with region API (the only allocator,
> providing aligned allocations) - it has separate reserve + alloc, and
> aligned_reserve + aligned_alloc.

On this argument... to use static alloc in mpstream, not aligning 
the return value is not enough.
You actually need something a batch alloc API, something 
like:
- static_alloc_begin_batch() - resets the buffer and sets the
position to 0,
- static_alloc_alloc_batch() - alloc memory, but return error if
  allocation would wrap around, since we need to make sure all
  allocation in a batch stay valid
- static_alloc_end_batch() - finish allocating memory within 
  a batch

Otherwise, how do you guarantee that a subsequent alloc used by
mpstream doesn't wrap the static buffer around and begins
overwriting it?


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