tarantool / avro-schema

Apache Avro schema tools for Tarantool
57 stars 4 forks source link

Consider replacing back fstack with debug.getlocal() #139

Open Totktonada opened 3 years ago

Totktonada commented 3 years ago

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. ```diff From 322196d5729b83b114af48e243487f1fe835ec56 Mon Sep 17 00:00:00 2001 From: Nick Zavaritsky 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 3 years ago

@Totktonada

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:

Seems it looks reasonable to use debug.getlocal() instead of fstack.