Re: [Tarantool-patches] [PATCH 1/1] box: fixed box.info:memory()

[Igor Munkin <imun@tarantool.org>]

Olya,

Thanks for your updates! The patch is OK, but I still have several
comments regarding it, so I pasted it below. Besides, the branch is
extremely outdated and based on 2.2 (it might complicate the merge
process).

| box: fix box.info:memory()
|
| Fix the return value of box.info:memory(). It used to return the same table
| as the box.info().
|
| If one removes metatable of the box.info:memory():
| tarantool> setmetatable(box.info:memory(), nil)
| ---
| - cache: 0
|   lua: 1855568
|   data: 37976
|   index: 1196032
|   net: 589824
|   tx: 0
|   version: 2.5.0-158-g667145aff
|   package: Tarantool
|
| While printing the table, lbox_info_memory_call adds box.info.memory's fields to
| the box.info table. To avoid this effect and return just box.info.memory we put
| an empty table on the Lua stack at the beginning of the lbox_info_memory_call
| function.
|
| Closes #4688

At first, the message still exceeds 72 symbols. Sasha has already
pointed to this several times. Let's make Sasha a bit happier!

Furthermore, you described why the output is the same as the one
obtained via box.info, but stripped everything regarding why box.info is
used here. I still guess it's worth to be mentioned, even if it is
perfectly described in Lua reference manual and PIL.

| diff --git a/src/box/lua/info.c b/src/box/lua/info.c
| index d0e553b1d..e5b1c32f0 100644
| --- a/src/box/lua/info.c
| +++ b/src/box/lua/info.c
| @@ -322,6 +322,8 @@ lbox_info_memory_call(struct lua_State *L)
|  	struct engine_memory_stat stat;
|  	engine_memory_stat(&stat);
|
| +	lua_newtable(L);
| +

I agree with Sasha, regarding <lua_createtable> usage: it can
preallocate the exact required size memory area, so I see no reasons to
ignore such benefit.

|  	lua_pushstring(L, "data");
|  	luaL_pushuint64(L, stat.data);
|  	lua_settable(L, -3);
| diff --git a/test/box-tap/gh-4688-box-info-memory.test.lua b/test/box-tap/gh-4688-box-info-memory.test.lua
| new file mode 100755
| index 000000000..8d6aa0e00
| --- /dev/null
| +++ b/test/box-tap/gh-4688-box-info-memory.test.lua
| @@ -0,0 +1,28 @@
| +#!/usr/bin/env tarantool
| +
| +--
| +-- gh-4688: box.info:memory() displayed full content of box.info
| +--
| +local tap = require('tap')
| +
| +box.cfg()
| +
| +local test = tap.test('gh-4688-box.info:memory-wrong-result')
| +test:plan(1)
| +
| +local a = box.info.memory()
| +local b = box.info:memory()

These variables are excess.

| +
| +local function get_keys(t)
| +    local keys = {}
| +    for k, v in pairs(t) do
| +        table.insert(keys, k)
| +    end
| +    return keys
| +end
| +
| +local keys_1 = get_keys(box.info.memory())
| +local keys_2 = get_keys(box.info:memory())
| +test:is_deeply(keys_1, keys_2, "box.info:memory coincide with box.info.memory")
| +
| +os.exit(test:check() and 0 or 1)

-- 
Best regards,
IM