Hi! Thank you for your answer!
Fixed codestyle and answered your question inline.

--
Serge Petrenko
sergepetrenko@tarantool.org

15 нояб. 2019 г., в 19:11, Alexander Turenko <alexander.turenko@tarantool.org> написал(а):

I have no any knowledge around fiber.top(), just asked a question
because of curiosity.

WBR, Alexander Turenko.

On Fri, Nov 15, 2019 at 05:58:58PM +0300, Serge Petrenko wrote:
We didn't refresh last remembered clock on fiber.top_enable()
This means that the fiber issuing fiber.top_enable() would get a huge
chunk of cpu time on its first yield. Fix this.
Also reset clock_delta and cpu_miss_count.

If fiber.top() is issued on the same ev loop iteration as
fiber.top_enable(), clock_delta_last is 0 for both cord and all the
fibers, so report "instant" and "average" stats per last iteration as 0
instead of NaN.

Follow-up #2694
---
src/lib/core/fiber.c | 21 +++++++++++++++++++++
src/lua/fiber.c      | 12 ++++++++++--
2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index aebaba7f0..258c094f5 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -1203,9 +1203,30 @@ fiber_top_enable()
ev_check_start(cord()->loop, &cord()->check_event);
fiber_top_enabled = true;

+ /*
+  * Reset cord and fiber clock stats in order to
+  * count from zero even on reenable.
+  */
cord()->clock_acc = 0;
cord()->cpu_miss_count_last = 0;
cord()->clock_delta_last = 0;
+ cord()->clock_delta = 0;
+
+ struct fiber * fiber;

Code style: struct fiber *fiber;

Fixed


+ rlist_foreach_entry(fiber, &cord()->alive, link) {
+ fiber->clock_acc = 0;
+ fiber->clock_delta_last = 0;
+ fiber->clock_delta = 0;
+ fiber->cputime = 0;
+ }
+
+ cord()->sched.clock_acc = 0;
+ cord()->sched.clock_delta_last = 0;
+ cord()->sched.clock_delta = 0;
+ cord()->sched.cputime = 0;
+
+ cord()->clock_last = __rdtscp(&cord()->cpu_id_last);
+ cord()->cpu_miss_count = 0;

Should not we doing such clean up in fiber_top_disable() rather then
here?

It doesn’t matter IMO. Well, yes, you don’t need a cleanup when you call
fiber.top_enable() for the first time, but, other than that, I see no difference.


struct timespec ts;
if (clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts) != 0) {
say_debug("clock_gettime(): failed to get this"
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 8b3b22e55..647505643 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -335,11 +335,19 @@ lbox_fiber_top_entry(struct fiber *f, void *cb_ctx)
lua_newtable(L);

lua_pushliteral(L, "average");
- lua_pushnumber(L, f->clock_acc / (double)cord()->clock_acc * 100);
+ if (cord()->clock_acc)
+ lua_pushnumber(L, f->clock_acc / (double)cord()->clock_acc * 100);
+ else
+ lua_pushnumber(L, 0);
lua_settable(L, -3);
+
lua_pushliteral(L, "instant");
- lua_pushnumber(L, f->clock_delta_last / (double)cord()->clock_delta_last * 100);
+ if (cord()->clock_delta_last)
+ lua_pushnumber(L, f->clock_delta_last / (double)cord()->clock_delta_last * 100);
+ else
+ lua_pushnumber(L, 0);
lua_settable(L, -3);

Code style: lines over 80 chars.

Fixed


+
lua_pushliteral(L, "time");
lua_pushnumber(L, f->cputime / (double) FIBER_TIME_RES);
lua_settable(L, -3);
-- 
2.21.0 (Apple Git-122)


diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 258c094f5..c6aab18b2 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -1212,7 +1212,7 @@ fiber_top_enable()
                cord()->clock_delta_last = 0;
                cord()->clock_delta = 0;
 
-               struct fiber * fiber;
+               struct fiber *fiber;
                rlist_foreach_entry(fiber, &cord()->alive, link) {
                        fiber->clock_acc = 0;
                        fiber->clock_delta_last = 0;
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 647505643..9268b0a19 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -335,17 +335,21 @@ lbox_fiber_top_entry(struct fiber *f, void *cb_ctx)
        lua_newtable(L);
 
        lua_pushliteral(L, "average");
-       if (cord()->clock_acc)
-               lua_pushnumber(L, f->clock_acc / (double)cord()->clock_acc * 100);
-       else
+       if (cord()->clock_acc) {
+               lua_pushnumber(L, f->clock_acc /
+                                 (double)cord()->clock_acc * 100);
+       } else {
                lua_pushnumber(L, 0);
+       }
        lua_settable(L, -3);
 
        lua_pushliteral(L, "instant");
-       if (cord()->clock_delta_last)
-               lua_pushnumber(L, f->clock_delta_last / (double)cord()->clock_delta_last * 100);
-       else
+       if (cord()->clock_delta_last) {
+               lua_pushnumber(L, f->clock_delta_last /
+                                 (double)cord()->clock_delta_last * 100);
+       } else {
                lua_pushnumber(L, 0);
+       }
        lua_settable(L, -3);
 
        lua_pushliteral(L, "time");