Skip to content

Add total stack usage to memory matrix. #223

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 1 commit into from
Jun 15, 2017

Conversation

studavekar
Copy link
Contributor

This change adds total stack usage which is sum of max stack used by each
thread.

@studavekar
Copy link
Contributor Author

@bridadan @geky can you review.

@@ -768,6 +770,10 @@ def exporter_memory_metrics_csv(test_result_ext, test_suite_properties=None):
for test_suite_name in test_results:
test = test_results[test_suite_name]

# If test didn't pass, memory stats doesn't make much sense
if test['single_test_result'] != TEST_RESULT_OK:
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

@studavekar Seems like this is pretty harmless to leave in anyway right? Maybe the test failed due to a stack overflow...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah true that is good candidate where if test fails we would want the results of stack. However for failing tests the memory data can't be truly trusted. If test fails at init_xxx() the data reported by it would smaller. So for reporting we are ignoring the fail tests.

Can take out this check so we get data in either case.

@studavekar studavekar force-pushed the stack_usage_total branch 2 times, most recently from a4c3532 to 43854de Compare May 25, 2017 00:16
Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

One little thing, but everything else looks good!

@@ -17,6 +17,8 @@
Author: Przemyslaw Wirkus <[email protected]>
"""

from mbed_test_api import TEST_RESULT_OK
Copy link
Contributor

Choose a reason for hiding this comment

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

One last nit pick! This line isn't necessary any more since you removed that last check. Mind pulling this out too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@studavekar studavekar force-pushed the stack_usage_total branch 2 times, most recently from 6523b8f to 29a1cdc Compare May 26, 2017 19:21
This change adds total stack usage which is sum of max stack used by each
thread.
@studavekar studavekar force-pushed the stack_usage_total branch from 29a1cdc to bb04c24 Compare May 26, 2017 22:20
for cur_thread_stack_info in thread_stack_info:
if cur_thread_stack_info['stack_size'] > max_thread_stack_size:
max_thread_stack_size = cur_thread_stack_info['stack_size']
max_thread = cur_thread_stack_info

max_stack_usage_total += cur_thread_stack_info['max_stack']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated it to correctly report max_stage_usage_total

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work!

@mazimkhan mazimkhan merged commit 06c0143 into ARMmbed:master Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants