Re: [tarantool-patches] [PATCH v2 1/1] schema:frommap() to create table or tuple

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Wed, Apr 04, 2018 at 07:35:54PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
> index 29a9aca..2723baf 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -386,6 +396,111 @@ static struct trigger on_alter_space_in_lua = {
>  	RLIST_LINK_INITIALIZER, box_lua_space_new_or_delete, NULL, NULL
>  };
>  
> +/**
> + * 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)
> +{

Is it really necessary? Can't you always pass space_id and use
space_cache_find, which already caches last space ptr?

> +	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;
> +}

> @@ -464,4 +579,12 @@ box_lua_space_init(struct lua_State *L)
>  	lua_pushnumber(L, VCLOCK_MAX);
>  	lua_setfield(L, -2, "REPLICA_MAX");
>  	lua_pop(L, 2); /* box, schema */
> +
> +	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);
> +	lua_pop(L, 1);

AFAIU this function might get pretty hot. Shouldn't we use FFI for
calling it?