Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH 1/1] netbox: optimize body decoding
@ 2018-05-14 11:41 Vladislav Shpilevoy
  2018-05-15 18:31 ` [tarantool-patches] " Vladislav Shpilevoy
  2018-05-15 18:38 ` Konstantin Osipov
  0 siblings, 2 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-14 11:41 UTC (permalink / raw)
  To: tarantool-patches; +Cc: kostja

Now netbox decodes body twice: MessagePack -> Lua, Lua -> tuples.
Lets do MessagePack -> tuples.

Part of #3333
---
Branch: https://github.com/tarantool/tarantool/tree/gh-3333-netbox-high-cpu
Issue: https://github.com/tarantool/tarantool/issues/3333

 src/box/lua/net_box.c   | 42 ++++++++++++++++++++++++++++++++++++++++
 src/box/lua/net_box.lua | 51 +++++++++++++++++++++----------------------------
 2 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 04fe70b03..3d9479283 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -38,6 +38,7 @@
 #include "box/iproto_constants.h"
 #include "box/lua/tuple.h" /* luamp_convert_tuple() / luamp_convert_key() */
 #include "box/xrow.h"
+#include "box/tuple.h"
 
 #include "lua/msgpack.h"
 #include "third_party/base64.h"
@@ -555,6 +556,46 @@ handle_error:
 	return 2;
 }
 
+/**
+ * Decode Tarantool response body.
+ * @param Lua stack[1] Raw MessagePack pointer.
+ * @param Lua stack[2] End of body pointer.
+ * @retval Decoded body.
+ */
+static int
+netbox_decode_body(struct lua_State *L)
+{
+	uint32_t ctypeid;
+	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
+	const char *end = *(const char **)luaL_checkcdata(L, 2, &ctypeid);
+	assert(mp_typeof(*data) == MP_MAP);
+	uint32_t map_size = mp_decode_map(&data);
+	/* Until 2.0 body has no keys except DATA. */
+	assert(map_size == 1);
+	for (uint32_t i = 0; i < map_size; ++i) {
+		uint32_t key = mp_decode_uint(&data);
+		if (key == IPROTO_DATA) {
+			uint32_t count = mp_decode_array(&data);
+			lua_createtable(L, count, 0);
+			struct tuple_format *format =
+				box_tuple_format_default();
+			for (uint32_t j = 0; j < count; ++j) {
+				const char *begin = data;
+				mp_next(&data);
+				struct tuple *tuple =
+					box_tuple_new(format, begin, data);
+				if (tuple == NULL)
+					luaT_error(L);
+				luaT_pushtuple(L, tuple);
+				lua_rawseti(L, -2, j + 1);
+			}
+		}
+	}
+	if (data != end)
+		return luaL_error(L, "invalid xrow length");
+	return 1;
+}
+
 int
 luaopen_net_box(struct lua_State *L)
 {
@@ -572,6 +613,7 @@ luaopen_net_box(struct lua_State *L)
 		{ "encode_auth",    netbox_encode_auth },
 		{ "decode_greeting",netbox_decode_greeting },
 		{ "communicate",    netbox_communicate },
+		{ "decode_body",    netbox_decode_body },
 		{ NULL, NULL}
 	};
 	/* luaL_register_module polutes _G */
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 46382129f..194a1c86b 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -27,7 +27,6 @@ local encode_auth     = internal.encode_auth
 local encode_select   = internal.encode_select
 local decode_greeting = internal.decode_greeting
 
-local sequence_mt      = { __serialize = 'sequence' }
 local TIMEOUT_INFINITY = 500 * 365 * 86400
 local VSPACE_ID        = 281
 local VINDEX_ID        = 289
@@ -51,30 +50,28 @@ local E_PROC_LUA             = box.error.PROC_LUA
 local is_final_state         = {closed = 1, error = 1}
 
 local function decode_nil(...) end
-local function decode_nop(...) return ... end
-local function decode_tuple(response)
-    if response[1] then
-        return box.tuple.new(response[1])
-    end
+local function decode_data(raw_data, raw_data_end)
+    local response, real_end = decode(raw_data)
+    assert(real_end == raw_data_end, "invalid xrow length")
+    return response[IPROTO_DATA_KEY]
+end
+local function decode_tuple(raw_data, raw_data_end)
+    return internal.decode_body(raw_data, raw_data_end)[1]
 end
-local function decode_get(response)
-    if response[2] then
+local function decode_get(raw_data, raw_data_end)
+    local body = internal.decode_body(raw_data, raw_data_end)
+    if body[2] then
         return nil, box.error.MORE_THAN_ONE_TUPLE
     end
-    if response[1] then
-        return box.tuple.new(response[1])
-    end
+    return body[1]
 end
-local function decode_select(response)
-    setmetatable(response, sequence_mt)
-    local tnew = box.tuple.new
-    for i, v in pairs(response) do
-        response[i] = tnew(v)
-    end
-    return response
+local function decode_select(raw_data, raw_data_end)
+    return internal.decode_body(raw_data, raw_data_end)
 end
-local function decode_count(response)
-    return response[1]
+local function decode_count(raw_data, raw_data_end)
+    local response, real_end = decode(raw_data)
+    assert(real_end == raw_data_end, "invalid xrow length")
+    return response[IPROTO_DATA_KEY][1]
 end
 
 local method_encoder = {
@@ -103,8 +100,8 @@ local method_encoder = {
 local method_decoder = {
     ping    = decode_nil,
     call_16 = decode_select,
-    call_17 = decode_nop,
-    eval    = decode_nop,
+    call_17 = decode_data,
+    eval    = decode_data,
     insert  = decode_tuple,
     replace = decode_tuple,
     delete  = decode_tuple,
@@ -115,7 +112,7 @@ local method_decoder = {
     min     = decode_get,
     max     = decode_get,
     count   = decode_count,
-    inject  = decode_nop,
+    inject  = decode_data,
 }
 
 local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
@@ -470,12 +467,8 @@ local function create_transport(host, port, user, password, callback,
         end
 
         -- Decode xrow.body[DATA] to Lua objects
-        body, body_end_check = decode(body_rpos)
-        assert(body_end == body_end_check, "invalid xrow length")
-        if body and body[IPROTO_DATA_KEY] then
-            request.response, request.errno =
-                method_decoder[request.method](body[IPROTO_DATA_KEY])
-        end
+        request.response, request.errno =
+            method_decoder[request.method](body_rpos, body_end)
         request.cond:broadcast()
     end
 
-- 
2.15.1 (Apple Git-101)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH 1/1] netbox: optimize body decoding
  2018-05-14 11:41 [tarantool-patches] [PATCH 1/1] netbox: optimize body decoding Vladislav Shpilevoy
@ 2018-05-15 18:31 ` Vladislav Shpilevoy
  2018-05-15 18:38 ` Konstantin Osipov
  1 sibling, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-15 18:31 UTC (permalink / raw)
  To: tarantool-patches; +Cc: kostja

Hello.

> Константин Осипов, [15 мая 2018 г., 21:16:08]:
> я всё же считаю что у decode_body должна быть такая же сигнатура как у decode
> 
> вот эта проверка должна быть снаружи:
> 
> +       if (data != end)
> +               return luaL_error(L, "invalid xrow length");
> 
> и сообщение кривое
> 
> это не xrow length
> 
> это body length

Below the fix is presented. At the end of the letter the whole patch is
attached.

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 3d9479283..adf5f7603 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -560,14 +560,13 @@ handle_error:
   * Decode Tarantool response body.
   * @param Lua stack[1] Raw MessagePack pointer.
   * @param Lua stack[2] End of body pointer.
- * @retval Decoded body.
+ * @retval Decoded body and position of the body end.
   */
  static int
  netbox_decode_body(struct lua_State *L)
  {
  	uint32_t ctypeid;
  	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
-	const char *end = *(const char **)luaL_checkcdata(L, 2, &ctypeid);
  	assert(mp_typeof(*data) == MP_MAP);
  	uint32_t map_size = mp_decode_map(&data);
  	/* Until 2.0 body has no keys except DATA. */
@@ -591,9 +590,8 @@ netbox_decode_body(struct lua_State *L)
  			}
  		}
  	}
-	if (data != end)
-		return luaL_error(L, "invalid xrow length");
-	return 1;
+	*(const char **)luaL_pushcdata(L, ctypeid) = data;
+	return 2;
  }
  
  int
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 194a1c86b..8fcaf89e8 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -49,29 +49,30 @@ local E_PROC_LUA             = box.error.PROC_LUA
  -- utility tables
  local is_final_state         = {closed = 1, error = 1}
  
-local function decode_nil(...) end
-local function decode_data(raw_data, raw_data_end)
-    local response, real_end = decode(raw_data)
-    assert(real_end == raw_data_end, "invalid xrow length")
-    return response[IPROTO_DATA_KEY]
+local function decode_nil(raw_data, raw_data_end)
+    return nil, raw_data_end
  end
-local function decode_tuple(raw_data, raw_data_end)
-    return internal.decode_body(raw_data, raw_data_end)[1]
+local function decode_data(raw_data)
+    local response, raw_end = decode(raw_data)
+    return response[IPROTO_DATA_KEY], raw_end
  end
-local function decode_get(raw_data, raw_data_end)
-    local body = internal.decode_body(raw_data, raw_data_end)
+local function decode_tuple(raw_data)
+    local response, raw_end = internal.decode_body(raw_data)
+    return response[1], raw_end
+end
+local function decode_get(raw_data)
+    local body, raw_end = internal.decode_body(raw_data)
      if body[2] then
-        return nil, box.error.MORE_THAN_ONE_TUPLE
+        return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
      end
-    return body[1]
+    return body[1], raw_end
  end
-local function decode_select(raw_data, raw_data_end)
-    return internal.decode_body(raw_data, raw_data_end)
+local function decode_select(raw_data)
+    return internal.decode_body(raw_data)
  end
-local function decode_count(raw_data, raw_data_end)
-    local response, real_end = decode(raw_data)
-    assert(real_end == raw_data_end, "invalid xrow length")
-    return response[IPROTO_DATA_KEY][1]
+local function decode_count(raw_data)
+    local response, raw_end = decode(raw_data)
+    return response[IPROTO_DATA_KEY][1], raw_end
  end
  
  local method_encoder = {
@@ -467,8 +468,10 @@ local function create_transport(host, port, user, password, callback,
          end
  
          -- Decode xrow.body[DATA] to Lua objects
-        request.response, request.errno =
+        local real_end
+        request.response, real_end, request.errno =
              method_decoder[request.method](body_rpos, body_end)
+        assert(real_end == body_end, "invalid body length")
          request.cond:broadcast()
      end
  
===========================================================================================

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 04fe70b03..adf5f7603 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -38,6 +38,7 @@
  #include "box/iproto_constants.h"
  #include "box/lua/tuple.h" /* luamp_convert_tuple() / luamp_convert_key() */
  #include "box/xrow.h"
+#include "box/tuple.h"
  
  #include "lua/msgpack.h"
  #include "third_party/base64.h"
@@ -555,6 +556,44 @@ handle_error:
  	return 2;
  }
  
+/**
+ * Decode Tarantool response body.
+ * @param Lua stack[1] Raw MessagePack pointer.
+ * @param Lua stack[2] End of body pointer.
+ * @retval Decoded body and position of the body end.
+ */
+static int
+netbox_decode_body(struct lua_State *L)
+{
+	uint32_t ctypeid;
+	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
+	assert(mp_typeof(*data) == MP_MAP);
+	uint32_t map_size = mp_decode_map(&data);
+	/* Until 2.0 body has no keys except DATA. */
+	assert(map_size == 1);
+	for (uint32_t i = 0; i < map_size; ++i) {
+		uint32_t key = mp_decode_uint(&data);
+		if (key == IPROTO_DATA) {
+			uint32_t count = mp_decode_array(&data);
+			lua_createtable(L, count, 0);
+			struct tuple_format *format =
+				box_tuple_format_default();
+			for (uint32_t j = 0; j < count; ++j) {
+				const char *begin = data;
+				mp_next(&data);
+				struct tuple *tuple =
+					box_tuple_new(format, begin, data);
+				if (tuple == NULL)
+					luaT_error(L);
+				luaT_pushtuple(L, tuple);
+				lua_rawseti(L, -2, j + 1);
+			}
+		}
+	}
+	*(const char **)luaL_pushcdata(L, ctypeid) = data;
+	return 2;
+}
+
  int
  luaopen_net_box(struct lua_State *L)
  {
@@ -572,6 +611,7 @@ luaopen_net_box(struct lua_State *L)
  		{ "encode_auth",    netbox_encode_auth },
  		{ "decode_greeting",netbox_decode_greeting },
  		{ "communicate",    netbox_communicate },
+		{ "decode_body",    netbox_decode_body },
  		{ NULL, NULL}
  	};
  	/* luaL_register_module polutes _G */
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 46382129f..8fcaf89e8 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -27,7 +27,6 @@ local encode_auth     = internal.encode_auth
  local encode_select   = internal.encode_select
  local decode_greeting = internal.decode_greeting
  
-local sequence_mt      = { __serialize = 'sequence' }
  local TIMEOUT_INFINITY = 500 * 365 * 86400
  local VSPACE_ID        = 281
  local VINDEX_ID        = 289
@@ -50,31 +49,30 @@ local E_PROC_LUA             = box.error.PROC_LUA
  -- utility tables
  local is_final_state         = {closed = 1, error = 1}
  
-local function decode_nil(...) end
-local function decode_nop(...) return ... end
-local function decode_tuple(response)
-    if response[1] then
-        return box.tuple.new(response[1])
-    end
+local function decode_nil(raw_data, raw_data_end)
+    return nil, raw_data_end
  end
-local function decode_get(response)
-    if response[2] then
-        return nil, box.error.MORE_THAN_ONE_TUPLE
-    end
-    if response[1] then
-        return box.tuple.new(response[1])
-    end
+local function decode_data(raw_data)
+    local response, raw_end = decode(raw_data)
+    return response[IPROTO_DATA_KEY], raw_end
  end
-local function decode_select(response)
-    setmetatable(response, sequence_mt)
-    local tnew = box.tuple.new
-    for i, v in pairs(response) do
-        response[i] = tnew(v)
+local function decode_tuple(raw_data)
+    local response, raw_end = internal.decode_body(raw_data)
+    return response[1], raw_end
+end
+local function decode_get(raw_data)
+    local body, raw_end = internal.decode_body(raw_data)
+    if body[2] then
+        return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
      end
-    return response
+    return body[1], raw_end
+end
+local function decode_select(raw_data)
+    return internal.decode_body(raw_data)
  end
-local function decode_count(response)
-    return response[1]
+local function decode_count(raw_data)
+    local response, raw_end = decode(raw_data)
+    return response[IPROTO_DATA_KEY][1], raw_end
  end
  
  local method_encoder = {
@@ -103,8 +101,8 @@ local method_encoder = {
  local method_decoder = {
      ping    = decode_nil,
      call_16 = decode_select,
-    call_17 = decode_nop,
-    eval    = decode_nop,
+    call_17 = decode_data,
+    eval    = decode_data,
      insert  = decode_tuple,
      replace = decode_tuple,
      delete  = decode_tuple,
@@ -115,7 +113,7 @@ local method_decoder = {
      min     = decode_get,
      max     = decode_get,
      count   = decode_count,
-    inject  = decode_nop,
+    inject  = decode_data,
  }
  
  local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
@@ -470,12 +468,10 @@ local function create_transport(host, port, user, password, callback,
          end
  
          -- Decode xrow.body[DATA] to Lua objects
-        body, body_end_check = decode(body_rpos)
-        assert(body_end == body_end_check, "invalid xrow length")
-        if body and body[IPROTO_DATA_KEY] then
-            request.response, request.errno =
-                method_decoder[request.method](body[IPROTO_DATA_KEY])
-        end
+        local real_end
+        request.response, real_end, request.errno =
+            method_decoder[request.method](body_rpos, body_end)
+        assert(real_end == body_end, "invalid body length")
          request.cond:broadcast()
      end
  
diff --git a/test/box/call.result b/test/box/call.result
index e27e2127d..40d7ef952 100644
--- a/test/box/call.result
+++ b/test/box/call.result
@@ -632,7 +632,7 @@ conn:eval("return return_sparse1()")
  ...
  conn:call_16("return_sparse1")
  ---
-- - [{20: 1, 1: 1}]
+- - [{1: 1, 20: 1}]
  ...
  function return_sparse2() return { [1] = 1, [20] = 1} end
  ---

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH 1/1] netbox: optimize body decoding
  2018-05-14 11:41 [tarantool-patches] [PATCH 1/1] netbox: optimize body decoding Vladislav Shpilevoy
  2018-05-15 18:31 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-05-15 18:38 ` Konstantin Osipov
  2018-05-15 18:42   ` Vladislav Shpilevoy
  1 sibling, 1 reply; 4+ messages in thread
From: Konstantin Osipov @ 2018-05-15 18:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/05/14 14:44]:
> Part of #3333
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gh-3333-netbox-high-cpu
> Issue: https://github.com/tarantool/tarantool/issues/3333
> 
>  src/box/lua/net_box.c   | 42 ++++++++++++++++++++++++++++++++++++++++
>  src/box/lua/net_box.lua | 51 +++++++++++++++++++++----------------------------
>  2 files changed, 64 insertions(+), 29 deletions(-)
> 
> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> index 04fe70b03..3d9479283 100644
> --- a/src/box/lua/net_box.c
> +++ b/src/box/lua/net_box.c
> @@ -38,6 +38,7 @@
>  #include "box/iproto_constants.h"
>  #include "box/lua/tuple.h" /* luamp_convert_tuple() / luamp_convert_key() */
>  #include "box/xrow.h"
> +#include "box/tuple.h"
>  
>  #include "lua/msgpack.h"
>  #include "third_party/base64.h"
> @@ -555,6 +556,46 @@ handle_error:
>  	return 2;
>  }
>  
> +/**
> + * Decode Tarantool response body.
> + * @param Lua stack[1] Raw MessagePack pointer.
> + * @param Lua stack[2] End of body pointer.
> + * @retval Decoded body.
> + */
> +static int
> +netbox_decode_body(struct lua_State *L)
> +{
> +	uint32_t ctypeid;
> +	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
> +	const char *end = *(const char **)luaL_checkcdata(L, 2, &ctypeid);
> +	assert(mp_typeof(*data) == MP_MAP);
> +	uint32_t map_size = mp_decode_map(&data);
> +	/* Until 2.0 body has no keys except DATA. */
> +	assert(map_size == 1);
> +	for (uint32_t i = 0; i < map_size; ++i) {
> +		uint32_t key = mp_decode_uint(&data);
> +		if (key == IPROTO_DATA) {
> +			uint32_t count = mp_decode_array(&data);
> +			lua_createtable(L, count, 0);
> +			struct tuple_format *format =
> +				box_tuple_format_default();
> +			for (uint32_t j = 0; j < count; ++j) {
> +				const char *begin = data;
> +				mp_next(&data);
> +				struct tuple *tuple =
> +					box_tuple_new(format, begin, data);
> +				if (tuple == NULL)
> +					luaT_error(L);
> +				luaT_pushtuple(L, tuple);
> +				lua_rawseti(L, -2, j + 1);
> +			}
> +		}
> +	}
> +	if (data != end)
> +		return luaL_error(L, "invalid xrow length");

The message is a bit misleading in net.box context. "Invalid
packet length" perhaps?

Besides, here you use luaL_error(), in other places in the code
you use assert(). Can we be consistent, and use Lua errors
instead, as long as you insist on keeping these checks? The
performance impact will be the same but we'll have a chance to
see/handle this message in production deployments.

Please add a comment why you insist on having this check.

Technically, this condition should never be true. net.box works
with tarantool, and tarantool always returns correct messagepack
(unless there is a bug of course). If the message pack is broken,
we can be PWN!ed, as the comment in decode() indicates. 

If you still insist on having these checks, please add a comment:
-- Keep the check for debug purposes: the condition may be true
-- only in case of Tarantool bug


> +	return 1;
> +}

Please let's have a consistent signature for decode() functions
and decode_body() has the same signature as decode()


The patch looks good overall besides these two issues.

Did you benchmark it? I'd expect a pretty hefty improvement, esp.
in case of large result sets. Care to run a test?

> +
>  int
>  luaopen_net_box(struct lua_State *L)
>  {
> @@ -572,6 +613,7 @@ luaopen_net_box(struct lua_State *L)
>  		{ "encode_auth",    netbox_encode_auth },
>  		{ "decode_greeting",netbox_decode_greeting },
>  		{ "communicate",    netbox_communicate },
> +		{ "decode_body",    netbox_decode_body },
>  		{ NULL, NULL}
>  	};
>  	/* luaL_register_module polutes _G */
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 46382129f..194a1c86b 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -27,7 +27,6 @@ local encode_auth     = internal.encode_auth
>  local encode_select   = internal.encode_select
>  local decode_greeting = internal.decode_greeting
>  
> -local sequence_mt      = { __serialize = 'sequence' }
>  local TIMEOUT_INFINITY = 500 * 365 * 86400
>  local VSPACE_ID        = 281
>  local VINDEX_ID        = 289
> @@ -51,30 +50,28 @@ local E_PROC_LUA             = box.error.PROC_LUA
>  local is_final_state         = {closed = 1, error = 1}
>  
>  local function decode_nil(...) end
> -local function decode_nop(...) return ... end
> -local function decode_tuple(response)
> -    if response[1] then
> -        return box.tuple.new(response[1])
> -    end
> +local function decode_data(raw_data, raw_data_end)
> +    local response, real_end = decode(raw_data)
> +    assert(real_end == raw_data_end, "invalid xrow length")
> +    return response[IPROTO_DATA_KEY]
> +end
> +local function decode_tuple(raw_data, raw_data_end)
> +    return internal.decode_body(raw_data, raw_data_end)[1]
>  end
> -local function decode_get(response)
> -    if response[2] then
> +local function decode_get(raw_data, raw_data_end)
> +    local body = internal.decode_body(raw_data, raw_data_end)
> +    if body[2] then
>          return nil, box.error.MORE_THAN_ONE_TUPLE
>      end
> -    if response[1] then
> -        return box.tuple.new(response[1])
> -    end
> +    return body[1]
>  end
> -local function decode_select(response)
> -    setmetatable(response, sequence_mt)
> -    local tnew = box.tuple.new
> -    for i, v in pairs(response) do
> -        response[i] = tnew(v)
> -    end
> -    return response
> +local function decode_select(raw_data, raw_data_end)
> +    return internal.decode_body(raw_data, raw_data_end)
>  end
> -local function decode_count(response)
> -    return response[1]
> +local function decode_count(raw_data, raw_data_end)
> +    local response, real_end = decode(raw_data)
> +    assert(real_end == raw_data_end, "invalid xrow length")
> +    return response[IPROTO_DATA_KEY][1]
>  end
>  
>  local method_encoder = {
> @@ -103,8 +100,8 @@ local method_encoder = {
>  local method_decoder = {
>      ping    = decode_nil,
>      call_16 = decode_select,
> -    call_17 = decode_nop,
> -    eval    = decode_nop,
> +    call_17 = decode_data,
> +    eval    = decode_data,
>      insert  = decode_tuple,
>      replace = decode_tuple,
>      delete  = decode_tuple,
> @@ -115,7 +112,7 @@ local method_decoder = {
>      min     = decode_get,
>      max     = decode_get,
>      count   = decode_count,
> -    inject  = decode_nop,
> +    inject  = decode_data,
>  }
>  
>  local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
> @@ -470,12 +467,8 @@ local function create_transport(host, port, user, password, callback,
>          end
>  
>          -- Decode xrow.body[DATA] to Lua objects
> -        body, body_end_check = decode(body_rpos)
> -        assert(body_end == body_end_check, "invalid xrow length")
> -        if body and body[IPROTO_DATA_KEY] then
> -            request.response, request.errno =
> -                method_decoder[request.method](body[IPROTO_DATA_KEY])
> -        end
> +        request.response, request.errno =
> +            method_decoder[request.method](body_rpos, body_end)
>          request.cond:broadcast()
>      end
>  
> -- 
> 2.15.1 (Apple Git-101)

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH 1/1] netbox: optimize body decoding
  2018-05-15 18:38 ` Konstantin Osipov
@ 2018-05-15 18:42   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-15 18:42 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches

Hello. I fixed your remarks already. See the previous letter in this thread.
> 
> Did you benchmark it? I'd expect a pretty hefty improvement, esp.
> in case of large result sets. Care to run a test?

One of our customers has benchmark. He opened the issue. But he uses netbox to
execute SQL requests, so he can bench, when the patch reaches 2.0.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-15 18:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-14 11:41 [tarantool-patches] [PATCH 1/1] netbox: optimize body decoding Vladislav Shpilevoy
2018-05-15 18:31 ` [tarantool-patches] " Vladislav Shpilevoy
2018-05-15 18:38 ` Konstantin Osipov
2018-05-15 18:42   ` Vladislav Shpilevoy

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