Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CPU Statistics #6857

Merged
merged 5 commits into from
May 22, 2018
Merged

CPU Statistics #6857

merged 5 commits into from
May 22, 2018

Conversation

deepikabhavnani
Copy link

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 cpu_stats branch 7 times, most recently from ba13bcb to 18c5d81 Compare May 11, 2018 15:29
@deepikabhavnani
Copy link
Author

Rebased - No PR dependency now

@deepikabhavnani deepikabhavnani mentioned this pull request May 11, 2018
Merged
@deepikabhavnani
Copy link
Author

CC @flit

@deepikabhavnani
Copy link
Author

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.

SenRamakri
SenRamakri previously approved these changes May 14, 2018
View reviewed changes
c1728p9
c1728p9 previously approved these changes May 14, 2018
View reviewed changes
rtos/rtos_idle.h Outdated
@@ -26,6 +26,7 @@
#define RTOS_IDLE_H

#include <stddef.h>
#include <stdint.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this header still needed?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch 👍 Will remove it

@deepikabhavnani deepikabhavnani dismissed stale reviews from c1728p9 and SenRamakri via 25c50d4 May 14, 2018 21:15
@0xc0170
Copy link
Contributor

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)

bulislaw
bulislaw reviewed May 15, 2018
View reviewed changes
hal/mbed_sleep_manager.c Outdated
return (sleep_time+deep_sleep_time);
}

uint64_t mbed_uptime(void)
Copy link
Member

Choose a reason for hiding this comment

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

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

bulislaw
bulislaw reviewed May 15, 2018
View reviewed changes
platform/mbed_power_mgmt.h Outdated
@@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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.

0xc0170
0xc0170 previously approved these changes May 16, 2018
View reviewed changes
@cmonr
Copy link
Contributor

cmonr commented May 16, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented May 16, 2018

Build : FAILURE

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

Updated test to not use event queue.
7900863 
Test was failing on limited RAM devices, because of shared event queue size.
Updated test to use thread instead of event queue.
@0xc0170
Copy link
Contributor

0xc0170 commented May 17, 2018

/morph build

@mbed-ci
Copy link

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
Copy link

mbed-ci commented May 17, 2018

Test : FAILURE

Build number : 1858
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6857/1858

@mbed-ci
Copy link

mbed-ci commented May 17, 2018

Exporter Build : SUCCESS

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

@deepikabhavnani
Copy link
Author

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

@cmonr
Copy link
Contributor

cmonr commented May 17, 2018

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

@0xc0170
Copy link
Contributor

0xc0170 commented May 19, 2018

/morph build

@mbed-ci
Copy link

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
Copy link

mbed-ci commented May 19, 2018

Exporter Build : SUCCESS

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

@mbed-ci
Copy link

mbed-ci commented May 19, 2018

Test : SUCCESS

Build number : 1881
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6857/1881

@adbridge
Copy link
Contributor

adbridge commented May 21, 2018 •
edited
Loading

Testing has passed but

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

@0xc0170 0xc0170 merged commit 5d027f4 into ARMmbed:master May 22, 2018
@cmonr
Copy link
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 cpu_stats branch May 23, 2018 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bulislaw bulislaw bulislaw left review comments

@kjbracey kjbracey kjbracey left review comments

@c1728p9 c1728p9 c1728p9 approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@SenRamakri SenRamakri SenRamakri left review comments

Assignees
No one assigned
Labels
release-version: 5.9.0-rc1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@deepikabhavnani @0xc0170 @bulislaw @cmonr @mbed-ci @adbridge @c1728p9 @kjbracey @SenRamakri