schedstats: revert to 32Bit, and enhance output of schedstats by smlng · Pull Request #7048 · RIOT-OS/RIOT

smlng · 2017-05-12T08:32:16Z

~~based on #6975~~

kaspar030 · 2017-05-16T08:27:18Z

As this touches the scheduler, we need to check for impact on context switch times. ping if you need test applications...

smlng · 2017-05-16T09:55:29Z

@kaspar030 ping 😉 I think this should also be tested for #6975? Because that introduces most of the changes, here I merely get rid of double and replace it by 2 unsigned ints.

cgundogan · 2017-06-01T07:15:36Z

needs a rebase

smlng · 2017-06-01T08:41:42Z

partly reverts #6975 back to 32Bit timer, gets rid of float in ps output, and enhances the ps_schedstats tests

vincent-d · 2017-06-01T08:51:50Z

core/include/sched.h

 *  @param[in] callback The callback functions the will be called
 */
-void sched_register_cb(void (*callback)(uint32_t, uint32_t));
+void sched_register_cb(void (*callback)(uint64_t, uint32_t));


First parameter could remain uint32_t as laststart is.

vincent-d · 2017-06-01T08:52:28Z

tests/ps_schedstatistics/main.c

 {
-    int next = (int)arg < NB_THREADS - 1 ? (int)arg + 1 : 0;
-    msg_t msg;
+    int next = ((int)arg +1) % NB_THREADS;


coding style: spaces around +

vincent-d · 2017-06-01T08:52:42Z

tests/ps_schedstatistics/main.c

+        msg_t m1, m2;
+        msg_receive(&m1);
+        /* generate differents loads per thead */
+        for (int i = 0; i < 10*(next+1); ++i) {


oops, again

vincent-d · 2017-06-01T08:52:52Z

tests/ps_schedstatistics/main.c

+        for (int i = 0; i < 10*(next+1); ++i) {
+            _xtimer_now64();
+        }
+        xtimer_usleep(XTIMER_BACKOFF*3);


smlng · 2017-06-01T09:02:52Z

comments addressed

vincent-d · 2017-06-01T09:37:31Z

Something is wrong with ratio calculation:

> ps
2017-06-01 11:34:53,398 - INFO #  ps
2017-06-01 11:34:53,412 - INFO # 	pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     | runtime  | switches
2017-06-01 11:34:53,425 - INFO # 	  - | isr_stack            | -        - |   - |   512 (  132) | 0x20000000 | 0x200001c8
2017-06-01 11:34:53,443 - INFO # 	  1 | idle                 | pending  Q |  15 |   256 (  128) | 0x20000744 | 0x200007c4  | 37.3733% |     99389
2017-06-01 11:34:53,462 - INFO # 	  2 | main                 | running  Q |   7 |  1536 (  768) | 0x20000844 | 0x20000c3c  | 65226.2620% |      4078
2017-06-01 11:34:53,482 - INFO # 	  3 | thread               | bl rx    _ |   6 |  1024 (  388) | 0x20000eec | 0x2000122c  | 10.1039% |     41529
2017-06-01 11:34:53,503 - INFO # 	  4 | thread               | bl rx    _ |   6 |  1024 (  388) | 0x200012ec | 0x2000162c  | 12.1230% |     41669
2017-06-01 11:34:53,520 - INFO # 	  5 | thread               | bl rx    _ |   6 |  1024 (  388) | 0x200016ec | 0x20001a2c  | 14.1449% |     41811
2017-06-01 11:34:53,539 - INFO # 	  6 | thread               | bl rx    _ |   6 |  1024 (  388) | 0x20001aec | 0x20001e2c  | 16.1640% |     41953
2017-06-01 11:34:53,559 - INFO # 	  7 | thread               | bl mutex _ |   6 |  1024 (  388) | 0x20001eec | 0x2000222c  |  8.8210% |     42094
2017-06-01 11:34:53,568 - INFO # 	    | SUM                  |            |     |  7424 ( 2968)

(This was on nucleo144-f413)

smlng · 2017-06-01T09:39:13Z

yeah, I there are some artifacts, I'm on it - will mark as WIP for the time being

smlng · 2017-06-01T09:54:49Z

I tested with native, where the values seem to be okay and the sum < 100%. However, I also tested on remote-revb, I don't see such huge values as you did, but the sum is constantly > 100% 🙁 weird ...

smlng · 2017-06-01T21:02:22Z

sys/ps/ps.c

+            unsigned runtime_major = (runtime_ticks * 100) / rt_sum;
+            unsigned runtime_minor = ((runtime_ticks % rt_sum) * 1000) / rt_sum;
+            /* add trailing zeros */
+            while(runtime_minor < 100) {


note to self: this shouldn't be necessary

smlng · 2017-06-02T09:26:43Z

some performance results using @kaspar030 bench_msg tool:

branch	stats	native	remote-revb
master	no	108000 (100%)	17951 (100%)
master	yes	38000 (35%)	12341 (69%)
this PR	yes	56000 (52%)	15032 (84%)

smlng · 2017-06-02T11:07:30Z

@kaspar030 your review in this one?

kaspar030 · 2017-06-02T21:30:57Z

core/sched.c

        schedstat *active_stat = &sched_pidlist[active_thread->pid];
        if (active_stat->laststart) {
-            active_stat->runtime_ticks += now - active_stat->laststart;
+            /* ensure now > laststart, to avoid flawed sum of runtime ticks


IMO we can remove this check. It doesn't catch all overflows anyways. Worse, it drops indeterministic periods depending on when the thread got scheduled.
I think that losing 71minute overflows, and documenting that properly, is preferable than this.

... losing 71minute overflows ...

It would mean that all running/sleeping periods would be counted module 2**32 xtimer ticks, which is ~71minutes. I doubt that it would have significant impact in ps output, compared to dropping all periods that happen to have an overflow.

mhm, how do you get 71 minutes? Doesn't that depend on the timer frequency - which is 1MHz for many boards but 250kHz for Ardunio Uno, for instance ...

I changed the calculation to be more generic, should be cool now 🤓

kaspar030 · 2017-06-08T13:07:52Z

core/sched.c

-            active_stat->runtime_ticks += now - active_stat->laststart;
-        }
+        /* add up runtime, overflow compatible */
+        active_stat->runtime_ticks += (((uint64_t)now + UINT32_MAX) -


what does this to compared to the simple difference?

öhm, actually nothing - I was thinking to complicated, need coffee ...

btw. reverted to simple difference

smlng · 2017-06-09T08:39:41Z

@kaspar030 addressed your comments, happy?

- use unsigned int instead of double

smlng · 2017-06-16T19:17:50Z

@kaspar030 do you mind having a look again?

aabadie · 2017-06-23T08:43:48Z

ping @kaspar030. This one is ready, only your ACK is missing.

smlng · 2017-07-02T20:43:34Z

long time no see, @kaspar030 anything to add here?

smlng · 2017-07-27T11:32:21Z

@kaspar030 still waiting for you to come back here and have a look

smlng · 2017-09-20T14:40:50Z

@kaspar030 this one hangs around for some time now, IIRC you weren't happy with the 64bit changes so why not revert and improve with this one?

smlng · 2017-09-28T13:42:34Z

and another ping @kaspar030, this (re)improves test performance as you wished 😉

smlng added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation State: waiting for other PR State: The PR requires another PR to be merged first labels May 12, 2017

smlng self-assigned this May 12, 2017

smlng force-pushed the enh/ps/schedstats branch 2 times, most recently from e6e3ce0 to 1346d3b Compare May 15, 2017 14:12

smlng mentioned this pull request May 15, 2017

ps: fix schedstatistics #6975

Merged

kaspar030 added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label May 16, 2017

smlng force-pushed the enh/ps/schedstats branch from 1346d3b to df4fb39 Compare May 16, 2017 10:16

smlng changed the title ~~[WIP] ps: enhance output of schedstats~~ ps: enhance output of schedstats May 16, 2017

smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 16, 2017

smlng force-pushed the enh/ps/schedstats branch from df4fb39 to 1e792c4 Compare June 1, 2017 08:38

smlng changed the title ~~ps: enhance output of schedstats~~ schedstats: revert to 32Bit, and enhance output of schedstats Jun 1, 2017

smlng force-pushed the enh/ps/schedstats branch from 1e792c4 to fd2b870 Compare June 1, 2017 08:42

smlng removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jun 1, 2017

smlng requested a review from kaspar030 June 1, 2017 08:42

vincent-d requested changes Jun 1, 2017

View reviewed changes

smlng force-pushed the enh/ps/schedstats branch from fd2b870 to cc3173f Compare June 1, 2017 09:02

smlng force-pushed the enh/ps/schedstats branch 4 times, most recently from 31096b7 to d6b670c Compare June 1, 2017 09:26

smlng added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 1, 2017

smlng commented Jun 1, 2017

View reviewed changes

smlng force-pushed the enh/ps/schedstats branch from 1af2e63 to d2426bc Compare June 2, 2017 09:05

kaspar030 requested changes Jun 2, 2017

View reviewed changes

schedstats: revert RIOT-OS#6975, use 32Bit again

e7136e2

smlng force-pushed the enh/ps/schedstats branch from d2426bc to 564f35f Compare June 8, 2017 12:53

kaspar030 reviewed Jun 8, 2017

View reviewed changes

smlng force-pushed the enh/ps/schedstats branch from 564f35f to 2b39fe1 Compare June 8, 2017 13:21

smlng added 2 commits June 9, 2017 10:50

schedstats: refine ps output

3ac6d6c

- use unsigned int instead of double

tests: enhance ps_schedstatistics

19351b5

smlng force-pushed the enh/ps/schedstats branch from 2b39fe1 to 19351b5 Compare June 9, 2017 08:50

aabadie added this to the Release 2017.07 milestone Jun 23, 2017

aabadie modified the milestones: Release 2017.07, Release 2017.10 Jun 30, 2017

kaspar030 approved these changes Sep 28, 2017

View reviewed changes

kaspar030 merged commit 5cc8204 into RIOT-OS:master Sep 28, 2017

smlng deleted the enh/ps/schedstats branch September 28, 2017 14:15

Conversation

smlng commented May 12, 2017 • edited by cgundogan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 commented May 16, 2017

Uh oh!

smlng commented May 16, 2017

Uh oh!

cgundogan commented Jun 1, 2017

Uh oh!

smlng commented Jun 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Jun 1, 2017

Uh oh!

vincent-d commented Jun 1, 2017

Uh oh!

smlng commented Jun 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smlng commented Jun 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Jun 2, 2017

Uh oh!

smlng commented Jun 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Jun 9, 2017

Uh oh!

smlng commented Jun 16, 2017

Uh oh!

aabadie commented Jun 23, 2017

Uh oh!

smlng commented Jul 2, 2017

Uh oh!

smlng commented Jul 27, 2017

Uh oh!

smlng commented Sep 20, 2017

Uh oh!

smlng commented Sep 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

smlng commented May 12, 2017 •

edited by cgundogan

Loading

smlng commented Jun 1, 2017 •

edited

Loading