Hello. See 7 comments below.
On 09/04/2018 11:30, Kirill Shcherbatov wrote:
I've accounted Kostya's comments.
Going to measure performance.
From ef47d3536b845a84b52be6c84a66aef12fbe8561 Mon Sep 17 00:00:00 2001
From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
Date: Mon, 9 Apr 2018 11:24:38 +0300
Subject: [PATCH] Moved obtaining cptr to Lua code.
---
src/box/lua/schema.lua | 33 +++++++++++++++++++++++++---
src/box/lua/space.cc | 59 ++++++--------------------------------------------
2 files changed, 37 insertions(+), 55 deletions(-)
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 323c2d6..a0f2d25 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1072,6 +1072,19 @@ end
internal.check_iterator_type = check_iterator_type -- export for net.box
+-- Get a C space structure pointer by space id using cache.
+-- Update cache if necessary.
+-- The _schema_version field is a cache invalidation rule.
+function space_cptr_cached(space)
+ local schema_version = builtin.box_schema_version()
+ if space._schema_version == schema_version then
+ return space._cptr
+ end
+ space._cptr = builtin.space_by_id(space.id)
+ space._schema_version = schema_version
+ return space._cptr
+end > +
function box.schema.space.bless(space)
local index_mt = {}
-- __len and __index
@@ -1320,7 +1333,7 @@ function box.schema.space.bless(space)
end
space_mt.bsize = function(space)
check_space_arg(space, 'bsize')
- local s = builtin.space_by_id(space.id)
+ local s = space_cptr_cached(space)
if s == nil then
box.error(box.error.NO_SUCH_SPACE, space.name)
end
@@ -1413,13 +1426,20 @@ function box.schema.space.bless(space)
end
space_mt.run_triggers = function(space, yesno)
check_space_arg(space, 'run_triggers')
- local s = builtin.space_by_id(space.id)
+ local s = space_cptr_cached(space)
if s == nil then
box.error(box.error.NO_SUCH_SPACE, space.name)
end
builtin.space_run_triggers(s, yesno)
end
- space_mt.frommap = box.internal.space.frommap
+ space_mt.frommap = function(space, map, opts)
+ -- update the cache
+ local cptr = space_cptr_cached(space)
+ if cptr == nil then
+ return box.NULL, "Space " .. space.name .. " does not exist"
+ end
+ return box.internal.space.frommap(space, map, opts)
+ end
space_mt.__index = space_mt
setmetatable(space, space_mt)
@@ -2234,5 +2254,12 @@ box.internal.schema = {}
box.internal.schema.init = function()
box_sequence_init()
end
+box.internal.space._cptr = function(space)
+ local cptr = space_cptr_cached(space)
+ if cptr == nil then
+ return box.NULL, "Space " .. space.name .. " does not exist"
+ end
+ return cptr
+end
box.NULL = msgpack.NULL
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 2723baf..9383096 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -397,51 +397,6 @@ static struct trigger on_alter_space_in_lua = {
};
/**
- * Get a C pointer for space structure.
- * Caches the result. Cleans the user(negative) stack itself.
- * @param Lua space object at top of the Lua stack.
- * @retval not nil C structure pointer.
- * @retval nil, err Can not find underlying space, error
- * description is returned.
- */
-static int
-lbox_space_ptr_cached(struct lua_State *L)
-{
- if (lua_gettop(L) < 1 || !lua_istable(L, 1))
- luaL_error(L, "Usage: space:_cptr()");
-
- lua_getfield(L, 1, "_schema_version");
- uint64_t schema_version = luaL_touint64(L, -1);
- lua_pop(L, 1);
- uint64_t global_shema_version = box_schema_version();
-
- void *space = NULL;
- if (schema_version == global_shema_version) {
- lua_getfield(L, 1, "_cptr");
- space = (void *)lua_topointer(L, -1);
- } else {
- lua_getfield(L, 1, "id");
- uint32_t id = luaL_touint64(L, -1);
- space = space_by_id(id);
- if (space == NULL) {
- lua_pushnil(L);
- lua_pushstring(L, tt_sprintf("Space with id '%d' "\
- "doesn't exist", id));
- return 2;
- }
-
- /** update the cache */
- luaL_pushuint64(L, global_shema_version);
- lua_setfield(L, 1, "_schema_version");
- lua_pushlightuserdata(L, space);
- lua_setfield(L, 1, "_cptr");
- }
- lua_pop(L, 1);
- lua_pushlightuserdata(L, space);
- return 1;
-}
-
-/**
* Make a tuple or a table Lua object by map.
* @param Lua space object.
* @param Lua map table object.
@@ -456,11 +411,11 @@ lbox_space_frommap(struct lua_State *L)
{
struct tuple_dictionary *dict = NULL;
struct space *space = NULL;
- int rc, argc = lua_gettop(L);
+ int argc = lua_gettop(L);
bool table = false;
if (argc < 2 || argc > 3 || !lua_istable(L, 2))
goto usage_error;
- if (argc == 3) {
+ if (argc == 3 && !lua_isnil(L, 3)) {
if (!lua_istable(L, 3))
goto usage_error;
lua_getfield(L, 3, "table");
@@ -469,10 +424,11 @@ lbox_space_frommap(struct lua_State *L)
table = lua_toboolean(L, -1);
}
- rc = lbox_space_ptr_cached(L);
- if (rc != 1)
- return rc;
- space = (struct space *) lua_topointer(L, -1);
+ lua_getfield(L, 1, "_cptr");
+ space = (struct space *)lua_topointer(L, -1);
+ /* validation is in the Lua code */
+ assert(space);
+
dict = space->format->dict;
lua_createtable(L, space->def->field_count, 0);
@@ -582,7 +538,6 @@ box_lua_space_init(struct lua_State *L)
static const struct luaL_Reg space_internal_lib[] = {
{"frommap", lbox_space_frommap},
- {"_cptr", lbox_space_ptr_cached},
{NULL, NULL}
};
luaL_register(L, "box.internal.space", space_internal_lib);
