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

CPU Statistics #6857

Merged
merged 5 commits into from May 22, 2018

Conversation

Projects
None yet
9 participants
@deepikabhavnani
Contributor

deepikabhavnani commented May 9, 2018

Description

API to get CPU stats like sleep/deepsleep time, uptime and idle time. These can be used by application to know the CPU Usage runtime.

Preceding PR dependency: ##6821

Status: In progress

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[X] Feature
[ ] Breaking change

CC @SenRamakri

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:cpu_stats branch 7 times, most recently from ba13bcb to 18c5d81 May 9, 2018

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented May 11, 2018

Rebased - No PR dependency now

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented May 14, 2018

CC @flit

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented May 14, 2018

Suggestion from @c1728p9 was to have single low power ticker and idle time can be calculated as sum of sleep and deep sleep timings. Will be compatible even with system having single thread.

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:cpu_stats branch from 57d1f35 to 43f9112 May 14, 2018

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:cpu_stats branch from 43f9112 to 908cfd3 May 14, 2018

@@ -26,6 +26,7 @@
#define RTOS_IDLE_H
#include <stddef.h>
#include <stdint.h>

This comment has been minimized.

@c1728p9

c1728p9 May 14, 2018

Contributor

Is this header still needed?

This comment has been minimized.

@deepikabhavnani

deepikabhavnani May 14, 2018

Contributor

Good catch 👍 Will remove it

@deepikabhavnani deepikabhavnani dismissed stale reviews from c1728p9 and SenRamakri via 25c50d4 May 14, 2018

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:cpu_stats branch from 908cfd3 to 25c50d4 May 14, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 15, 2018

Can you review astyle travis check - few style changes requested there.

We'll have a look why it is not reported here (as +ZZ warnings)

return (sleep_time+deep_sleep_time);
}
uint64_t mbed_uptime(void)

This comment has been minimized.

@bulislaw

bulislaw May 15, 2018

Member

mbed_uptime is the same as read_us can we remove one? or at least make one simply call the other?

@@ -205,6 +205,32 @@ static inline void system_reset(void)
{
NVIC_SystemReset();
}
/** Provides the time spent in sleep mode, since system is up and running

This comment has been minimized.

@bulislaw

bulislaw May 15, 2018

Member

since system is up and running that's a bit confusing, can just say from boot? Also we should call out in each of the functions (@note) it only works if platform supports LP ticker.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 16, 2018

/morph build

@cmonr cmonr added needs: CI and removed needs: review labels May 16, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented May 16, 2018

Updated test to not use event queue.
Test was failing on limited RAM devices, because of shared event queue size.
Updated test to use thread instead of event queue.

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:cpu_stats branch from 8c9e036 to 7900863 May 16, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 17, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented May 17, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented May 17, 2018

Test failed on NUCLEO_F746ZG, adding additional read to take care of lazy initialization of lp ticker.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 17, 2018

@deepikabhavnani Will restart in a bit. Giving PRs targeted for 5.8.5 a chance to complete.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 19, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented May 19, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@adbridge

This comment has been minimized.

Contributor

adbridge commented May 21, 2018

Testing has passed but

@0xc0170 @bulislaw @c1728p9 @kjbracey-arm this still needs review approval from 2 of you!

@adbridge adbridge added needs: review and removed needs: CI labels May 21, 2018

@0xc0170 0xc0170 merged commit 5d027f4 into ARMmbed:master May 22, 2018

13 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, 846 warnings (+1 warnings)
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 8695 cycles (-994 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label May 23, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 23, 2018

FYI, somehow this test build was magic because two other PRs have failed with this update (#6910 (comment)) (#6847 (comment)).

@deepikabhavnani has a fix ready here (#6993), and it's been locally tested multiple times without failure. Conversely, if master is used, it tends to fail quite often. The issue is possibly due to CI load.

In the future, we should probably consider seriously running multiple test builds back to back when new or updated tests are involved to make sure they get heavy testing on them.

@deepikabhavnani deepikabhavnani deleted the deepikabhavnani:cpu_stats branch May 23, 2018

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