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.

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

tests: add stack usage metrics #17706

Merged
merged 9 commits into from
Mar 29, 2022

Conversation

kaspar030
Copy link
Contributor

@kaspar030 kaspar030 commented Feb 25, 2022

Contribution description

This PR adds a module that, when enabled, will make threads print their stack usage on exit, in a format that the CI's metrics collection understands (and is somewhat human readable).

Example:

main(): This is RIOT! (Version: 2022.04-devel-501-g310e0-add_stack_usage_metrics)
If you can read this:                                                                                                                              
4294967295                                                               
-2147483648
FA
-2147483648
12345678
123456789ABCDEF0
18446744073709551615
-9223372036854775808
1.23450
Test successful.
{ "stack usage": { "main": 376 }}

This has some impact, the printing itself uses printf. Also, the actual stack data fields are kept per-thread, so 8 bytes more per thread. That means unfortunately some tests for some small-ram boards fall over the cliff.

Testing procedure

CI should be fine.

Issues/PRs references

@github-actions github-actions bot added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: sys Area: System Area: tests Area: tests and testing framework labels Feb 25, 2022
@kaspar030
Copy link
Contributor Author

this might still have some DEVELHELP related issues.

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 24, 2022
@github-actions github-actions bot added the Area: Kconfig Area: Kconfig integration label Mar 25, 2022
@kaspar030
Copy link
Contributor Author

Mmmh, the metric collection fails when an application runs multiple threads with the same name (or, restarts one of them). :(

I'm trying to come up with a solution that doesn't require the metrics collection script to deal with metric specific cases, like, if metric=="stack usage".

Would creating a list make sense, if there's multiple entries? that would look like this:

   691         "tests/malloc_thread_safety": {                                                                                                     
   692             "native:gnu": {                                                                                                                 
   693                 "stack usage": {                                                                                                            
   694                     "idle": {                                                                                                               
   695                         "size": 8192,                                                                                                       
   696                         "used": 600                                                                                                         
   697                     },                                                                                                                      
   698                     "main": {                                                                                                               
   699                         "size": 12288,                                                                                                      
   700                         "used": 2508                                                                                                        
   701                     },                                                                                                                      
   702                     "t1": {                                                                                                                 
   703                         "size": 4096,                                                                                                       
   704                         "used": [                                                                                                           
   705                             860,                                                                                                            
   706                             472                                                                                                             
   707                         ]                                                                                                                   
   708                     },                                                                                                                      
   709                     "t2": {                                                                                                                 
   710                         "size": 4096,                                                                                                       
   711                         "used": [                                                                                                           
   712                             860,                                                                                                            
   713                             472                                                                                                             
   714                         ]                                                                                                                   
   715                     }                                                                                                                       
   716                 }                                                                                                                           
   717             }                                                                                                                               
   718         },

(note the multiple "used" entries for t1 and t2)

@github-actions github-actions bot added the Area: CI Area: Continuous Integration of RIOT components label Mar 27, 2022
@kaspar030
Copy link
Contributor Author

OK, I went with the threads being a list:

    1 {                                                                                                                                            
    2     "metrics": {                                                                                                                             
    3         "tests/malloc_thread_safety": {                                                                                                      
    4             "native:gnu": {                                                                                                                  
    5                 "threads": [                                                                                                                 
    6                     {                                                                                                                        
    7                         "name": "t1",                                                                                                        
    8                         "stack_size": 4096,                                                                                                  
    9                         "stack_used": 860                                                                                                    
   10                     },                                                                                                                       
   11                     {                                                                                                                        
   12                         "name": "t2",                                                                                                        
   13                         "stack_size": 4096,                                                                                                  
   14                         "stack_used": 860                                                                                                    
   15                     },                                                                                                                       
   16                     {                                                                                                                        
   17                         "name": "t1",                                                                                                        
   18                         "stack_size": 4096,                                                                                                  
   19                         "stack_used": 472                                                                                                    
   20                     },                                                                                                                       
   21                     {                                                                                                                        
   22                         "name": "t2",                                                                                                        
   23                         "stack_size": 4096,                                                                                                  
   24                         "stack_used": 472                                                                                                    
   25                     },                                                                                                                       

this needs a fix to the metrics script so lists get concatenated.

@kaspar030 kaspar030 added 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 Mar 28, 2022
@MrKevinWeiss MrKevinWeiss added 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 Mar 28, 2022
@MrKevinWeiss
Copy link
Contributor

results after the murdock scripts have been updated

{
    "metrics": {
        "tests/malloc_thread_safety": {
            "native:gnu": {
                "threads": [
                    {
                        "name": "t1",
                        "stack_size": 4096,
                        "stack_used": 860
                    },
                    {
                        "name": "t2",
                        "stack_size": 4096,
                        "stack_used": 860
                    },
                    {
                        "name": "t1",
                        "stack_size": 4096,
                        "stack_used": 472
                    },
                    {
                        "name": "t2",
                        "stack_size": 4096,
                        "stack_used": 472
                    },
                    {
                        "name": "idle",
                        "stack_size": 8192,
                        "stack_used": 600
                    },
                    {
                        "name": "main",
                        "stack_size": 12288,
                        "stack_used": 2508
                    }
                ]
            }
        },
        "tests/thread_cooperation": {
            "native:gnu": {
                "threads": [
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2508
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2412
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2412
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2412
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2412
                    },
                    {
                        "name": "thread",
                        "stack_size": 8192,
                        "stack_used": 2412
                    },
                    {
                        "name": "idle",
                        "stack_size": 8192,
                        "stack_used": 436
                    },
                    {
                        "name": "main",
                        "stack_size": 12288,
                        "stack_used": 2612
                    }
                ]
            }
        }
    }
}

@kaspar030 kaspar030 added the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Mar 28, 2022
@kaspar030 kaspar030 force-pushed the add_stack_usage_metrics branch 2 times, most recently from 7161055 to c64a776 Compare March 28, 2022 08:13
@kaspar030 kaspar030 removed the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Mar 28, 2022
@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Mar 28, 2022
@kaspar030 kaspar030 added 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 Mar 28, 2022
@kaspar030
Copy link
Contributor Author

I think this is good to go!

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Code looks good to me and test output was provided. Please squash the style nitpick right in.

sys/test_utils/print_stack_usage/print_stack_usage.c Outdated Show resolved Hide resolved
This commit adds a module that can print stack metrics in the format as
understood by CI.
- activate THREAD_CREATE_STACKTEST also if test_utils_print_stack_usage
  is used

- make thread_measure_stack_free() available unconditionally

- if DEVELHELP is active, call test_utils_print_stack_usage() on any
  thread exit

- if DEVELHELP is active, call test_utils_print_stack_usage() after main
  for the idle thread, if that is used
@kaspar030 kaspar030 merged commit fd7c185 into RIOT-OS:master Mar 29, 2022
@kaspar030 kaspar030 deleted the add_stack_usage_metrics branch March 29, 2022 21:09
@kaspar030
Copy link
Contributor Author

Thanks!

@fjmolinas
Copy link
Contributor

tests/pthread_flood is failing on samr21-xpro after this PR, I'm guessing stack usage? Could you take a look @kaspar030 ?

47dd3b1889c813550c1029d03c06c404a29d42cb is the first bad commit
commit 47dd3b1889c813550c1029d03c06c404a29d42cb
Author: Kaspar Schleiser <kaspar@schleiser.de>
Date:   Thu Aug 6 13:27:13 2020 +0200

    tests: make test_utils_print_stack_usage a default module

 tests/Makefile.tests_common | 2 ++
 1 file changed, 2 insertions(+)

@kaspar030
Copy link
Contributor Author

on it

@kaspar030
Copy link
Contributor Author

Hm. The pthread threads' stack is too small for the printing of the stack sizes :/

@fjmolinas
Copy link
Contributor

Hm. The pthread threads' stack is too small for the printing of the stack sizes :/

DISABLE_MODULE then?

@kaspar030
Copy link
Contributor Author

Using fmt if available, don't print anything if the stack is too small: #17891

@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: Kconfig Area: Kconfig integration Area: sys Area: System 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants