Re: [Tarantool-patches] [PATCH 2/2] app/fiber: wait till a full event loop iteration ends.

["Sergey Petrenko" <sergepetrenko@tarantool.org>]

Hi! Thank you for review!

>Четверг, 14 ноября 2019, 1:45 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
>
>Hi! Thanks for the path!
>
>We don't use dots in commit title. I guess this is a typo. 

Fixed.

>
>
>On 13/11/2019 19:04, Serge Petrenko wrote:
>> fiber.top() fills in statistics every event loop iteration,
>> so if it was just enabled, fiber.top() may contain 'inf's and
>> 'nan's in fiber cpu usage averages because total time consumed by
>> the main thread was not yet accounted for.
>> Same stands for viewing top() results for a freshly created fiber:
>> its metrics will be zero since it hasn't lived a full ev loop iteration
>> yet.
>> Fix this by delaying the test till top() results are meaningful and add
>> minor refactoring.
>> 
>> Follow-up #2694
>> ---
>>  test/app/fiber.result   | 35 ++++++++++++++++++++++++++---------
>>  test/app/fiber.test.lua | 31 +++++++++++++++++++++++--------
>>  2 files changed, 49 insertions(+), 17 deletions(-)
>> 
>> diff --git a/test/app/fiber.result b/test/app/fiber.result
>> index 4a094939f..d447a36fc 100644
>> --- a/test/app/fiber.result
>> +++ b/test/app/fiber.result
>> @@ -1469,6 +1469,19 @@ sum = 0
>>  fiber.top_enable()
>>  ---
>>  ...
>> +-- Check that a number is finite.
>> +-- Lua doesn't have this, sorry.
>> +function finite(num)\
>> +    return type(num) == 'number' and num < math.huge and\
>> +           num > -math.huge and tostring(num) ~= 'nan'\
>
>1. A canonical way to check for NaN is compare a value with
>itself. If they are not equal, then it is NaN.

Fixed, thanks!

>
>
>But more important questions are:
>- How can a number from top() have a not 'number' type? 

It can't. I just wanted to implement a caconical is_finite check.
I can remove it, if you want me to.

>
>- How can top() contain a NaN, and an infinite value? 

NaN: you issue fiber.top() on the same iteration you called
fiber.top_enable(). cord()->clock_delta_last and fiber()->clock_delta_last
both are 0, because clock_delta_last contains data from a previous ev loop
iteration. Division gives you NaN.

>
>
>> +end
>> +---
>> +...
>> +-- Wait till a full event loop iteration passes, so that
>> +-- top() contains meaningful results.
>> +while not finite(fiber.top().cpu["1/sched"].instant) do  fiber.yield() end
>
>2. Double whitespace after 'do'.

Fixed, thanks.

Incremental diff follows.

diff --git a/test/app/fiber.result b/test/app/fiber.result
index d447a36fc..8eb506bd5 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1469,17 +1469,14 @@ sum = 0
 fiber.top_enable()
 ---
 ...
--- Check that a number is finite.
--- Lua doesn't have this, sorry.
-function finite(num)\
-    return type(num) == 'number' and num < math.huge and\
-           num > -math.huge and tostring(num) ~= 'nan'\
+function isnan(num)\
+    return num ~= num\
 end
 ---
 ...
 -- Wait till a full event loop iteration passes, so that
 -- top() contains meaningful results.
-while not finite(fiber.top().cpu["1/sched"].instant) do  fiber.yield() end
+while isnan(fiber.top().cpu["1/sched"].instant) do fiber.yield() end
 ---
 ...
 a = fiber.top()
diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
index 33ebe063f..24cb8b492 100644
--- a/test/app/fiber.test.lua
+++ b/test/app/fiber.test.lua
@@ -634,16 +634,13 @@ sum = 0
 -- gh-2694 fiber.top()
 fiber.top_enable()
 
--- Check that a number is finite.
--- Lua doesn't have this, sorry.
-function finite(num)\
-    return type(num) == 'number' and num < math.huge and\
-           num > -math.huge and tostring(num) ~= 'nan'\
+function isnan(num)\
+    return num ~= num\
 end
 
 -- Wait till a full event loop iteration passes, so that
 -- top() contains meaningful results.
-while not finite(fiber.top().cpu["1/sched"].instant) do  fiber.yield() end
+while isnan(fiber.top().cpu["1/sched"].instant) do fiber.yield() end
 
 a = fiber.top()
 type(a)
 


Regards,
Sergey Petrenko