[tarantool-patches] Re: [PATCH 1/1] netbox: optimize body decoding

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

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
  ---