mbedmicro-rtos-mbed tests : reduce memory consumption #4890
Conversation
|
Just a random suggestion: IMO |
|
Hi
This is the number of parallel thread in the test.
malloc test is currently FAILED for NUCLEO_F070RB and NUCLEO_F072RB only with GCC. |
| @@ -21,7 +21,7 @@ | |||
| #error [NOT_SUPPORTED] test not supported | |||
| #endif | |||
|
|
|||
| #define NUM_THREADS 5 | |||
There was a problem hiding this comment.
The large number of threads was specifically chosen to help this test catch race conditions faster and more reliably. When I was testing this with only 2 threads, race conditions wouldn't reliably get caught in the 15 seconds the test ran for. If you want to reduce the number of threads then you'll probably need to increase the test time.
You can verify this test is working by temporarily removing the malloc locks (__malloc_lock and __malloc_unlock) in platform/mbed_retarget.cpp and running this test and waiting for a failure. You should be able to run this test a couple of times with two threads to get the mean time to failure. From that you could then adjust the test time accordingly.
There was a problem hiding this comment.
Hi
I'm back...
Temp test done (removing malloc locks): either test is failing before first LED blink, either test is OK whatever the duration...
There was a problem hiding this comment.
Keep us updated (as we understand the test is failing?)
There was a problem hiding this comment.
New commit done.
Test report added.
There was a problem hiding this comment.
It can be fix in other way than thread number reduction, by removing extraneous heap allocations (by statically allocation of threads stack). PR #5338
...
uint8_t stack[THREAD_STACK_SIZE * NUM_THREADS];
...
// Allocate threads for the test
for (int i = 0; i < NUM_THREADS; i++) {
thread_list[i] = new Thread(osPriorityNormal, THREAD_STACK_SIZE, stack + i * THREAD_STACK_SIZE);
|
@jeromecoutant Any update for this patch? |
|
Test results with small OS5 STM32 targets: | target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
|
/morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
@jeromecoutant This caused a regression for 2 platforms: They result in timeout |
OK, the only impact I can see for the threads test is the thread size as I have reduced it from 768 to 512. Can I propose patch like: #if defined(TARGET_NUCLEO_F070RB) || defined(TARGET_NUCLEO_F072RB) |
|
Please review again, the code was updated. The test case now includes the proposal that was shared in the previous msg above. |
| @@ -25,7 +25,11 @@ | |||
| #error [NOT_SUPPORTED] test not supported | |||
| #endif | |||
|
|
|||
| #if defined(TARGET_NUCLEO_F070RB) || defined(TARGET_NUCLEO_F072RB) | |||
There was a problem hiding this comment.
The rest of changes are fine. I am not certain about this. Are we expecting to have here ifdef for varios targets .
There is not 2x768bytes available for these 2 targets?
Description
Hi
This proposition doesn't affect test goal.
It only reduces memory consumption
Thx
Status
READY