From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id F383C6EC57; Sat, 20 Mar 2021 03:50:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F383C6EC57 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1616201428; bh=boZdlXiurvuB+6hitPC4oSyuGDUzdhjSqz35/VY4tu8=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=EZOPKkLkLF5vMkyLc/CnsJMUqqDsdTqD2E8iQ8aEyXC9CTH1E/65mZEiLV31GZvUO gf1xFzs23egBxa5BudFfU4JQLiABKU5N5WeJCQo5S+uztXGMeaBGeRDdQFABZNq/WE PHXoZDXRtPAnRvxyz/n/ebeDWHlWeBv94gahQToU= Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 7597C71824 for ; Sat, 20 Mar 2021 03:43:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7597C71824 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1lNPhw-00076U-N1; Sat, 20 Mar 2021 03:43:13 +0300 To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org Date: Sat, 20 Mar 2021 01:42:45 +0100 Message-Id: X-Mailer: git-send-email 2.24.3 (Apple Git-128) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD95D6E7CC48CB1F5F179C48A9DDACBFB6F5347129BC2C9341C182A05F53808504079BE0E90EDF0C8460A9382A74F9906CAF2DC4E8BBB57B0D4E5A0C0A7726688CE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE789066434B85BF7C7EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637C1CDCB5E4A85220F8638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C81A23F326053F827A514491D23DC3368BF7D251B72F91DEEA471835C12D1D9774AD6D5ED66289B5278DA827A17800CE7A6779F98BF527B7A9FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3E3800B164E348C91117882F4460429728AD0CFFFB425014E868A13BD56FB6657A7F4EDE966BC389F9E8FC8737B5C2249753C3A5E0A5AB5B7089D37D7C0E48F6CCF19DD082D7633A0E7DDDDC251EA7DABAAAE862A0553A39223F8577A6DFFEA7C6898EBFCB5D2929543847C11F186F3C5E7DDDDC251EA7DABCC89B49CDF41148F90DBEB212AEC08F22623479134186CDE6BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C81A23F326053F827A514491D23DC3368BF7D251B72F91DEE9C2B6934AE262D3EE7EAB7254005DCED114C52B35DBB74F4E7EAB7254005DCEDEC0BC6DB5D5D7B8B1E0A4E2319210D9B64D260DF9561598F01A9E91200F654B0BDBC62CB1440F3B18E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D95739AEDB3821B45907BE1B6F6956BDBC8A0C71C30AE90CC58107D54C5D392C1100DC3BA59F412A1D7E09C32AA3244C8D184B05728A5A90C0E60ECD1B8074B933C9DC155518937FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj8xHC0Ak4ylakEJ/selx8cg== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822DBA76E8BD64F5D634E5786385AC312FF3841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH 08/16] uuid: replace static_alloc with ffi stash X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" static_alloc() appears not to be safe to use in Lua, because it does not provide any ownership protection for the returned values. The problem appears when something is allocated, then Lua GC starts, and some __gc handlers might also use static_alloc(). In Lua and in C - both lead to the buffer being corrupted in its original usage place. The patch is a part of activity of getting rid of static_alloc() in Lua. It removes it from uuid Lua module and makes it use the new FFI stash feature, which helps to cache frequently used and heavy to allocate FFI values. ffi.new() is not used, because - It produces a new GC object; - ffi.new('struct tt_uuid') costs around 300ns while FFI stash costs around 0.8ns (although it is magically fixed when ffi.new('struct tt_uuid[1]') is used); - Without JIT ffi.new() costs about the same as the stash, ~280ns for small objects like tt_uuid. The patch makes uuid perf a bit better in the places where static_alloc() was used, because its cost was around 7ns for one allocation. --- src/lua/uuid.lua | 102 ++++++++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 40 deletions(-) diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua index 9cebf3ace..74f8c924c 100644 --- a/src/lua/uuid.lua +++ b/src/lua/uuid.lua @@ -1,7 +1,7 @@ -- uuid.lua (internal file) local ffi = require("ffi") -local static_alloc = require('buffer').static_alloc +local buffer = require('buffer') local builtin = ffi.C ffi.cdef[[ @@ -27,6 +27,9 @@ extern const struct tt_uuid uuid_nil; local uuid_t = ffi.typeof('struct tt_uuid') local UUID_STR_LEN = 36 local UUID_LEN = ffi.sizeof(uuid_t) +local uuid_stash = buffer.ffi_stash_new(uuid_t) +local uuid_stash_take = uuid_stash.take +local uuid_stash_put = uuid_stash.put local is_uuid = function(value) return ffi.istype(uuid_t, value) @@ -66,10 +69,12 @@ local uuid_tobin = function(uu, byteorder) return error('Usage: uuid:bin([byteorder])') end if need_bswap(byteorder) then - local uuidbuf = static_alloc('struct tt_uuid') + local uuidbuf = uuid_stash_take() ffi.copy(uuidbuf, uu, UUID_LEN) builtin.tt_uuid_bswap(uuidbuf) - return ffi.string(ffi.cast('char *', uuidbuf), UUID_LEN) + uu = ffi.string(ffi.cast('char *', uuidbuf), UUID_LEN) + uuid_stash_put(uuidbuf) + return uu end return ffi.string(ffi.cast('char *', uu), UUID_LEN) end @@ -93,42 +98,29 @@ local uuid_isnil = function(uu) return builtin.tt_uuid_is_nil(uu) end -local fromstr = function(str) - local uu = static_alloc('struct tt_uuid') - local rc = builtin.tt_uuid_from_string(str, uu) - if rc ~= 0 then - return nil - end - return uu -end - -local to_uuid = function(value) - if is_uuid(value) then - return value - end - if type(value) == 'string' then - return fromstr(value) - end - return nil -end - -local check_uuid = function(value, index, err_lvl) - value = to_uuid(value) - if value ~= nil then - return value - end - +local function error_convert_arg(arg_index) local err_fmt = 'incorrect value to convert to uuid as %d argument' - error(err_fmt:format(index), err_lvl) + error(err_fmt:format(arg_index), 3) end local uuid_eq = function(lhs, rhs) - rhs = to_uuid(rhs) - if rhs == nil then + if not is_uuid(lhs) then + lhs, rhs = rhs, lhs + elseif is_uuid(rhs) then + return builtin.tt_uuid_is_equal(lhs, rhs) + end + if type(rhs) ~= 'string' then return false end - lhs = check_uuid(lhs, 1, 3) - return builtin.tt_uuid_is_equal(lhs, rhs) + local buf = uuid_stash_take() + local rc = builtin.tt_uuid_from_string(rhs, buf) == 0 + if rc then + rc = builtin.tt_uuid_is_equal(lhs, buf) + else + rc = false + end + uuid_stash_put(buf) + return rc end local uuid_new = function() @@ -138,20 +130,50 @@ local uuid_new = function() end local uuid_new_bin = function(byteorder) - local uuidbuf = static_alloc('struct tt_uuid') + local uuidbuf = uuid_stash_take() builtin.tt_uuid_create(uuidbuf) - return uuid_tobin(uuidbuf, byteorder) + local res = uuid_tobin(uuidbuf, byteorder) + uuid_stash_put(uuidbuf) + return res end local uuid_new_str = function() - local uuidbuf = static_alloc('struct tt_uuid') + local uuidbuf = uuid_stash_take() builtin.tt_uuid_create(uuidbuf) - return uuid_tostring(uuidbuf) + local res = uuid_tostring(uuidbuf) + uuid_stash_put(uuidbuf) + return res end local uuid_cmp = function(lhs, rhs) - lhs = check_uuid(lhs, 1, 4) - rhs = check_uuid(rhs, 2, 4) - return builtin.tt_uuid_compare(lhs, rhs) + local buf + if is_uuid(lhs) then + if is_uuid(rhs) then + -- Fast path. + return builtin.tt_uuid_compare(lhs, rhs) + end + if type(rhs) ~= 'string' then + return error_convert_arg(2) + end + buf = uuid_stash_take() + if builtin.tt_uuid_from_string(rhs, buf) ~= 0 then + uuid_stash_put(buf) + return error_convert_arg(2) + end + rhs = buf + else + if type(lhs) ~= 'string' then + return error_convert_arg(1) + end + buf = uuid_stash_take() + if builtin.tt_uuid_from_string(lhs, buf) ~= 0 then + uuid_stash_put(buf) + return error_convert_arg(1) + end + lhs = buf + end + local rc = builtin.tt_uuid_compare(lhs, rhs) + uuid_stash_put(buf) + return rc end local uuid_lt = function(lhs, rhs) return uuid_cmp(lhs, rhs) < 0 -- 2.24.3 (Apple Git-128)