8000 tests: add some benchmarks by kaspar030 · Pull Request #7786 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

tests: add some benchmarks #7786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 16, 2018
Merged

Conversation

kaspar030
Copy link
Contributor
@kaspar030 kaspar030 commented Oct 21, 2017

This PR adds some benchmark applications that I've been using for benchmarking RIOT's IPC performance.

@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tests Area: tests and testing framework labels Oct 21, 2017
@kaspar030 kaspar030 force-pushed the add_some_benchmarks branch from c73f05e to e68e70a Compare October 21, 2017 21:45
xtimer_t timer;
timer.callback = _timer_callback;

unsigned n = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering: could this lead to overflows on 16-bit platforms? I would imagine a decently fast clocked msp430 can do more than 65k context switches per s, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same in the other tests below...

n++;
}
flag = 0;
printf("Test complete. n=%u\n", n);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make the output a little more verbose, as in "Test complete: X context switches per second" or similar?

Also: if I am not mistaken, we do 2 * n context switches per second, correct?

n++;
}
flag = 0;
printf("Test complete. n=%u\n", n);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here (and below)

@haukepetersen
Copy link
Contributor

I know you gonna hate this: but I think for completeness there should be a README.md for each test, right?

@kYc0o kYc0o mentioned this pull request Oct 30, 2017
11 tasks
Copy link
Contributor
@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since #7871 is done (all mentioned tests there now have an associated testrunner script), it would be great to add test scripts here as well.

@haukepetersen
Copy link
Contributor

@kaspar030 in the process of advancing with #7445 I noticed that it would be very useful to have these benchmark applications to get a better feeling about any potential run-time side effects. I would like to help, too :-)

What I would also like to add, is some run-time measurements for certain API function calls, e.g. measure the time it takes to call a million times mutex_lock(); mutex_unlock(); or thread_flags_set() in a loop (similar to the benchmark in the periph_gpio test). For doing this, I see some options:

  • we could introduce a separate app for every module (mutex, thread_flags, ...) -> quite a few apps
  • we could add those additional tests to the fitting bench_ apps introduced in this PR -> but it would mean we mix run-time tests for function calls with run-time tests for context switching
  • we could refactor all ctx-switch test apps into a single app (possibly re-using stack memory), and adding a second app for testing the function call run-time

What do you think is the most feasible? As said: I will gladly do or help with any possibly needed refactoring/cleanup.

@kaspar030 kaspar030 force-pushed the add_some_benchmarks branch 2 times, most recently from 1f56f0f to d2af1a2 Compare June 14, 2018 10:24
@kaspar030
Copy link
Contributor Author
  • renamed tests
  • added README.md files
  • rebased

@kaspar030
Copy link
Contributor Author

we could introduce a separate app for every module (mutex, thread_flags, ...) -> quite a few apps
we could add those additional tests to the fitting bench_ apps introduced in this PR -> but it would mean we mix run-time tests for function calls with run-time tests for context switching
we could refactor all ctx-switch test apps into a single app (possibly re-using stack memory), and adding a second app for testing the function call run-time

(as discussed offline) I'd like to keep the context switch tests of this PR as is in order to be able to compare code size of these simple use cases over time.

@haukepetersen
Copy link
Contributor

@aabadie seems to me like all issues are addressed, do you give green light for merging this PR now?

@kaspar030
Copy link
Contributor Author

Pushed a note explaining code duplication. This is ready for review, @danpetry @haukepetersen wanna take a look?

@kaspar030
Copy link
Contributor Author

@aabadie are the tests to your liking?

Copy link
Contributor
@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good from my side.

Copy link
Contributor
@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found minor stuff here.

@@ -0,0 +1,12 @@
include ../Makefile.tests_common

BOARD_INSUFFICIENT_MEMORY := nucleo32-f031
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be nucleo-f031k6 now



def testfunc(child):
child.expect_exact(u"{ \"result\" :")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the script is python 3 only, there's no need for the u

@@ -0,0 +1,12 @@
include ../Makefile.tests_common

BOARD_INSUFFICIENT_MEMORY := nucleo32-f031
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same problem with nucleo board name



def testfunc(child):
child.expect_exact(u"{ \"result\" :")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u

@@ -0,0 +1,12 @@
include ../Makefile.tests_common

BOARD_INSUFFICIENT_MEMORY := nucleo32-f031
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nucleo



def testfunc(child):
child.expect_exact(u"{ \"result\" :")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u

@@ -0,0 +1,13 @@
include ../Makefile.tests_common

BOARD_INSUFFICIENT_MEMORY := nucleo32-f031
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nucleo, I stop here

Copy link
Contributor
@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, please squash!

@kaspar030 kaspar030 force-pushed the add_some_benchmarks branch from 1c7776b to 43bb05d Compare June 15, 2018 08:16
@kaspar030 kaspar030 added CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 15, 2018
@kaspar030 kaspar030 force-pushed the add_some_benchmarks branch from 2332b06 to 3c9eb94 Compare June 15, 2018 21:05
@kaspar030
Copy link
Contributor Author
  • fixed (and immediately squashed) the tests

(they worked previously, but the match ended like this: ```{ "result" : ````(cutting the rest of the line). Now the expect line includes the whole line, so the actual result shows up in the test run's output.

@aabadie maybe re-ack?

@aabadie
Copy link
Contributor
aabadie commented Jun 16, 2018

maybe re-ack?

ACK, and go

@aabadie aabadie merged commit 9946066 into RIOT-OS:master Jun 16, 2018
@kaspar030 kaspar030 deleted the add_some_benchmarks branch June 18, 2018 19:32
@cladmi cladmi added this to the Release 2018.07 milestone Jul 10, 2018
@jnohlgard jnohlgard mentioned this pull request Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0