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

Add CPU stats for greentea tests #7294

Merged
merged 1 commit into from Jun 30, 2018

Conversation

Projects
None yet
8 participants
@jeromecoutant
Contributor

jeromecoutant commented Jun 21, 2018

Description

Here is a proposition to add CPU info at the end of each greentea tests.

[1529585766.11][CONN][RXD] {{__cpu_info up time;1779541}}
[1529585766.15][CONN][RXD] {{__cpu_info sleep time;4516}}
[1529585766.19][CONN][RXD] {{__cpu_info deepsleep time;931397}}
[1529585766.22][CONN][RXD] {{__cpu_info % sleep / deep;0;49}}

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[x] Feature
[ ] Breaking change
@screamerbg

This comment has been minimized.

Member

screamerbg commented Jun 21, 2018

I like this! Do we have some way to measure tick counts and use the data as performance measurement? @0xc0170

@0xc0170 0xc0170 requested review from ARMmbed/mbed-os-tools, OPpuolitaival and studavekar Jun 21, 2018

@cmonr

I -really- like this.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 21, 2018

@studavekar Thoughts?

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 21, 2018

@screamerbg I think the people that could best answer that are @SenRamakri and/or @deepikabhavnani

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Jun 21, 2018

I like this! Do we have some way to measure tick counts and use the data as performance measurement?

Performance in terms of what?
We have way to calculate the CPU load (https://github.com/ARMmbed/mbed-os-example-cpu-usage/blob/master/main.cpp) which can be compared across different targets and application.

With the help of deep sleep / sleep comparison you can compare to see if deep sleep will help in battery saving (need to do some target specific analysis for that) - May be in future

greentea_send_kv("__cpu_info up time", mbed_uptime());
greentea_send_kv("__cpu_info sleep time", mbed_time_sleep());
greentea_send_kv("__cpu_info deepsleep time", mbed_time_deepsleep());
greentea_send_kv("__cpu_info % sleep / deep", (mbed_time_sleep() * 100) / mbed_uptime(), (mbed_time_deepsleep() * 100) / mbed_uptime());

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Jun 21, 2018

Contributor

(mbed_time_deepsleep() * 100) / mbed_uptime())

Not sure how useful this stat is, it says how much % time system spent in sleep since it was up. But will be interesting to see its result for different test application.

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Jun 21, 2018

This would be a good addition to provide additional cpu information, It would useful for teams to check device sleep status during the test execution. 👍

@cmonr cmonr added needs: CI and removed needs: review labels Jun 21, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 21, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 21, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 21, 2018

Darn. Looks like it's not going to be that simple and easy.

@jeromecoutant, please take a look at the build failures.

@cmonr cmonr added needs: work and removed needs: CI labels Jun 21, 2018

#if defined(MBED_CPU_STATS_ENABLED)
static void send_CPU_info()
{
greentea_send_kv("__cpu_info up time", mbed_uptime());

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Jun 21, 2018

Contributor

mbed_uptime , mbed_time_sleep , mbed_time_deepsleep are not API's. They are internal functions in Mbed OS.
Please use mbed_stats_cpu_get to get all CPU related info

This comment has been minimized.

@0xc0170

0xc0170 Jun 27, 2018

Member

@deepikabhavnani We might want to add that detail to the documentation? These functions are declared in power management header file - thus I would also use it . I do not see any internal detail mentioned there.

If they are internal, why are there declared then in the power m. header file (we got there _internal functions - intention is clear).

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Jun 27, 2018

Contributor

@0xc0170 - I was not completely clear with my comment, those functions can be used if required. But since this PR is about adding CPU stats for testing it will be good to use actual API mbed_stats_cpu_get and not the functions called by API.

This comment has been minimized.

@0xc0170

0xc0170 Jun 27, 2018

Member

Understood . Still confusing to a user - which one to use? Is it fine as it is, all those functions exposed ?

@jeromecoutant Can you update to use mbed_stats_cpu_get rather?

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Jun 27, 2018

Contributor

User can use mbed_uptime just to know since when the system is up and running (ok to have public). mbed_stats_cpu_get will give all the cpu related information. We have plans to expand this API to have additional information and parts of which may not be in power management header file.

@0xc0170

We need to be clear about mbed_uptime vs mbed_stats.

@deepikabhavnani Please address the comment about those functions. Once done, this should be good to go

@0xc0170

@jeromecoutant Can you update to use mbed_stats_cpu_get rather?

@jeromecoutant jeromecoutant dismissed stale reviews from OPpuolitaival, studavekar, and cmonr via 35c9ddc Jun 28, 2018

@jeromecoutant jeromecoutant force-pushed the jeromecoutant:PR_GREENTEA_STAT branch from ebd8655 to 35c9ddc Jun 28, 2018

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jun 28, 2018

@0xc0170 Update is done

@0xc0170 0xc0170 added needs: review and removed needs: work labels Jun 28, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 28, 2018

Considering that previous set was approved (similar to the current one, just one change there), starting CI

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 28, 2018

Build : SUCCESS

Build number : 2471
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7294/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 28, 2018

/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 29, 2018

/morph export-build

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit f10bb88 into ARMmbed:master Jun 30, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 929 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10191 cycles (+120 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@jeromecoutant jeromecoutant deleted the jeromecoutant:PR_GREENTEA_STAT branch Jul 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment