Consider replacing back fstack with debug.getlocal()
Totktonada opened this issue · 1 comments
Totktonada commented
It may give some performance on success paths.
We replaced debug.getlocal()
approach with fstack in order to fix #11. Later, @mejedi investigated the problem further and proposed a work around.
Workaround.
From 322196d5729b83b114af48e243487f1fe835ec56 Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky <mejedi@gmail.com>
Date: Fri, 28 Sep 2018 16:25:42 +0000
Subject: [PATCH] Bugfix
---
avro_schema/frontend.lua | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/avro_schema/frontend.lua b/avro_schema/frontend.lua
index 10f8ae4..e9dabbd 100644
--- a/avro_schema/frontend.lua
+++ b/avro_schema/frontend.lua
@@ -568,17 +568,14 @@ copy_data = function(schema, data, visited)
if data ~= null then
error()
end
- return null
elseif schematype == 'boolean' then
if type(data) ~= 'boolean' then
error()
end
- return data
elseif schematype == 'int' then
if data < -2147483648 or data > 2147483647 or floor(tonumber(data)) ~= data then
error()
end
- return data
elseif schematype == 'long' then
local n = tonumber(data)
-- note: if it's not a number or cdata(numbertype),
@@ -596,24 +593,20 @@ copy_data = function(schema, data, visited)
error()
end
end
- return data
elseif schematype == 'double' or schema == 'float' then
- return 0 + tonumber(data)
+ data = 0 + tonumber(data)
elseif schematype == 'bytes' or schematype == 'string' then
if type(data) ~= 'string' then
error()
end
- return data
elseif schematype == 'enum' then
if not get_enum_symbol_map(schema)[data] then
error()
end
- return data
elseif schematype == 'fixed' then
if type(data) ~= 'string' or #data ~= schema.size then
error()
end
- return data
else
if visited[data] then
error('@Infinite loop detected in the data', 0)
@@ -689,8 +682,11 @@ copy_data = function(schema, data, visited)
assert(false)
end
visited[data] = nil
- return res
+ data = res
end
+ goto l
+::l::
+ return data
end
-- extract from the call stack a path to the fragment that failed
--
2.17.1
Let's compare performance and decide.
olegrok commented
There is a test branch for this purpose - https://github.com/tarantool/avro-schema/tree/139-fstack-replace
I've run "benchmark.lua" before and after this commit. My results:
- Before: 0.247 M RPS
- After: 0.270 M RPS
Seems it looks reasonable to use debug.getlocal() instead of fstack.