From 3b1de78db8a1da9200f1ff56b99576a50d52a533 Mon Sep 17 00:00:00 2001
From: Nikolay Shirokovskiy <nshirokovskiy@tarantool.org>
Date: Fri, 22 Sep 2023 10:47:26 +0300
Subject: [PATCH] mpstream: get rid of mpstream_reset
Proposed ASAN implementation of region allocator does not support double
reservation for the sake of simplicity. Every reservation is supposed to
be followed by one or more allocations.
This restiction does not work well with mpstream currently. The issue is
mpstream_init/mpstream_reserve do reservation of size 0. For example In
case of region slab of min order is reserved (a chunk of memory of page
size currently). If the first data we want to write to mpstream is
larger then the reservation done then we make reservation again.
Let's get rid of this reservation at the beginning as it is suboptimal
behaviour. Moreover let's get rid of mpstream_reset as mpstream_init
is lightweight and we can create a new mpstream instead of reusing
exiting.
Also while we at it avoid allocation of 0 size in mpstream_flush as it
is done in mpstream_reserve_slow (see 3.0.0-alpha3-19-g8159347d0 "misc:
avoid allocations of size 0 for region" for details).
NO_TEST=internal
NO_CHANGELOG=internal
NO_DOC=internal
---
src/lib/mpstream/mpstream.c | 17 +++--------------
src/lib/mpstream/mpstream.h | 6 ++----
test/unit/lua_msgpack.c | 15 ++++++++-------
3 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/src/lib/mpstream/mpstream.c b/src/lib/mpstream/mpstream.c
index 3cff2dad96..f397dee0b3 100644
--- a/src/lib/mpstream/mpstream.c
+++ b/src/lib/mpstream/mpstream.c
@@ -40,19 +40,6 @@ mpstream_reserve_slow(struct mpstream *stream, size_t size)
stream->end = stream->pos + size;
}
-void
-mpstream_reset(struct mpstream *stream)
-{
- size_t size = 0;
- stream->buf = stream->reserve(stream->ctx, &size);
- if (stream->buf == NULL) {
- diag_set(OutOfMemory, size, "mpstream", "reset");
- stream->error(stream->error_ctx);
- }
- stream->pos = stream->buf;
- stream->end = stream->pos + size;
-}
-
/**
* A streaming API so that it's possible to encode to any output
* stream.
@@ -67,7 +54,9 @@ mpstream_init(struct mpstream *stream, void *ctx,
stream->alloc = alloc;
stream->error = error;
stream->error_ctx = error_ctx;
- mpstream_reset(stream);
+ stream->buf = NULL;
+ stream->pos = NULL;
+ stream->end = NULL;
}
void
diff --git a/src/lib/mpstream/mpstream.h b/src/lib/mpstream/mpstream.h
index 9fdd0c2aba..456b407f04 100644
--- a/src/lib/mpstream/mpstream.h
+++ b/src/lib/mpstream/mpstream.h
@@ -57,9 +57,6 @@ mpstream_panic_cb(void *error_ctx);
void
mpstream_reserve_slow(struct mpstream *stream, size_t size);
-void
-mpstream_reset(struct mpstream *stream);
-
/**
* A streaming API so that it's possible to encode to any output
* stream.
@@ -72,7 +69,8 @@ mpstream_init(struct mpstream *stream, void *ctx,
static inline void
mpstream_flush(struct mpstream *stream)
{
- stream->alloc(stream->ctx, stream->pos - stream->buf);
+ if (stream->pos != stream->buf)
+ stream->alloc(stream->ctx, stream->pos - stream->buf);
stream->buf = stream->pos;
}
diff --git a/test/unit/lua_msgpack.c b/test/unit/lua_msgpack.c
index ab8a996e60..fd6d194d99 100644
--- a/test/unit/lua_msgpack.c
+++ b/test/unit/lua_msgpack.c
@@ -61,7 +61,6 @@ test_encode_ext(lua_State *L)
"UUID is correctly encoded as MP_EXT");
ok(type == MP_EXT, "type of UUID is MP_EXT");
ibuf_reset(ibuf);
- mpstream_reset(&stream);
cord_ibuf_drop(ibuf);
mh_strnu32_delete(translation);
@@ -91,12 +90,12 @@ test_translation_in_encoding(lua_State *L)
struct ibuf *ibuf = cord_ibuf_take();
struct mpstream stream;
- mpstream_init(&stream, ibuf, ibuf_reserve_cb, ibuf_alloc_cb,
- mpstream_error_mock, L);
lua_createtable(L, 0, 1);
lua_pushboolean(L, true);
lua_setfield(L, 1, alias);
+ mpstream_init(&stream, ibuf, ibuf_reserve_cb, ibuf_alloc_cb,
+ mpstream_error_mock, L);
luamp_encode_with_translation(L, luaL_msgpack_default, &stream, 1,
translation, NULL);
lua_pop(L, 1);
@@ -104,13 +103,14 @@ test_translation_in_encoding(lua_State *L)
ok(strncmp(ibuf->buf, "\x81\x00\xc3", ibuf_used(ibuf)) == 0,
"first-level MP_MAP key is translated");
ibuf_reset(ibuf);
- mpstream_reset(&stream);
lua_createtable(L, 0, 1);
lua_createtable(L, 0, 1);
lua_pushboolean(L, true);
lua_setfield(L, -2, alias);
lua_setfield(L, -2, "k");
+ mpstream_init(&stream, ibuf, ibuf_reserve_cb, ibuf_alloc_cb,
+ mpstream_error_mock, L);
luamp_encode_with_translation(L, luaL_msgpack_default, &stream, 1,
translation, NULL);
lua_pop(L, 1);
@@ -118,19 +118,19 @@ test_translation_in_encoding(lua_State *L)
ok(strncmp(ibuf->buf, "\x81\xa1k\x81\xa1x\xc3", ibuf_used(ibuf)) == 0,
"only first-level MP_MAP key is translated");
ibuf_reset(ibuf);
- mpstream_reset(&stream);
lua_createtable(L, 0, 1);
lua_pushnumber(L, 0);
lua_pushboolean(L, true);
lua_settable(L, -3);
+ mpstream_init(&stream, ibuf, ibuf_reserve_cb, ibuf_alloc_cb,
+ mpstream_error_mock, L);
luamp_encode_with_translation(L, luaL_msgpack_default, &stream, 1,
translation, NULL);
mpstream_flush(&stream);
ok(strncmp(ibuf->buf, "\x81\x00\xc3", ibuf_used(ibuf)) == 0,
"only keys with MP_STRING type are translated");
ibuf_reset(ibuf);
- mpstream_reset(&stream);
lua_createtable(L, 0, 1);
lua_pushboolean(L, true);
@@ -138,6 +138,8 @@ test_translation_in_encoding(lua_State *L)
lua_pushnumber(L, 0);
lua_pushboolean(L, false);
lua_settable(L, -3);
+ mpstream_init(&stream, ibuf, ibuf_reserve_cb, ibuf_alloc_cb,
+ mpstream_error_mock, L);
luamp_encode_with_translation(L, luaL_msgpack_default, &stream, 1,
translation, NULL);
lua_pop(L, 1);
@@ -147,7 +149,6 @@ test_translation_in_encoding(lua_State *L)
"MP_MAP key that is the value of the translation are translated "
"correctly");
ibuf_reset(ibuf);
- mpstream_reset(&stream);
cord_ibuf_drop(ibuf);
mh_strnu32_delete(translation);
--
GitLab