<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Igor!</p>
<p>I have fixed all issue you have mentioned and rebased and updated
branch.</p>
<p>Since there is no much have changed I will write here only new
commit message:</p>
<pre class="text-gray ws-pre-wrap" style="box-sizing: border-box; font-family: SFMono-Regular, Consolas, "Liberation Mono", Menlo, monospace; font-size: 12px; margin-top: 0px; margin-bottom: 0px; color: rgb(88, 96, 105) !important; white-space: pre-wrap; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-style: initial; text-decoration-color: initial;">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
When box.info:memory is called, it has box.info table as a first
argument (see <a href="https://www.lua.org/manual/5.1/manual.html#2.8" rel="nofollow" style="box-sizing: border-box; background-color: initial; color: rgb(3, 102, 214); text-decoration: none;">https://www.lua.org/manual/5.1/manual.html#2.8</a>). 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. Instead adding fields to box.info
table lbox_info_memory_call fills that empty table.
<span class="issue-keyword tooltipped tooltipped-se" aria-label="This commit closes issue #4688." style="box-sizing: border-box; position: relative; border-bottom: 1px dotted rgb(149, 157, 165);">Closes</span> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="539037443" data-permission-text="Title is private" data-url="https://github.com/tarantool/tarantool/issues/4688" data-hovercard-type="issue" data-hovercard-url="/tarantool/tarantool/issues/4688/hovercard" href="https://github.com/tarantool/tarantool/issues/4688" style="box-sizing: border-box; background-color: initial; color: rgb(3, 102, 214); text-decoration: none;">#4688</a>
</pre>
<div class="moz-cite-prefix">16.07.2020 20:56, Igor Munkin пишет:<br>
</div>
<blockquote type="cite"
cite="mid:20200716175618.GN5559@tarantool.org">
<pre class="moz-quote-pre" wrap="">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)
</pre>
</blockquote>
</body>
</html>