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

malloc test refactoring #5323

Merged
merged 1 commit into from
Nov 22, 2017
Merged

Conversation

maciejbocianski
Copy link
Contributor

@maciejbocianski maciejbocianski commented Oct 16, 2017

Description

Test refactoring
Few test added: malloc(0), free(NULL), malloc(large size)

Status

READY

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 17, 2017

cc @c1728p9 @bulislaw

Copy link
Contributor

@c1728p9 c1728p9 left a comment

Choose a reason for hiding this comment

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

This should be split into two separate PRs. One which adds the additional tests and the other which reduces test memory consumption.

@bulislaw
Copy link
Member

Or at least commits

@maciejbocianski
Copy link
Contributor Author

out of memory fix moved to #5338

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 19, 2017

Update posted, please review

extern uint32_t mbed_heap_size;
static const int test_timeout = 25;
volatile bool thread_should_continue = true;
#define NUM_THREADS 4
Copy link
Contributor

Choose a reason for hiding this comment

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

Did the number of threads need to be decreased to pass testing?

Copy link
Contributor Author

@maciejbocianski maciejbocianski Oct 19, 2017

Choose a reason for hiding this comment

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

We could restore back thread count to 5, but it will cause fail on GCC_ARM.
To run 5 threads on GCC_ARM we have to free additional ~300B of heap memory.

To achieve this we could reduce thread allocation to 20B or move part of Thread objects on stack (but can not move all of them because we reach main thread stack limit).

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do 5 threads no longer fit?

Copy link
Contributor Author

@maciejbocianski maciejbocianski Oct 25, 2017

Choose a reason for hiding this comment

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

it turns out that refactoring (new test cases, functions, ...) increased static RAM usage by 480B as a result less memory remains for heap and fifth thread stack allocation fails

python tools/memap.py BUILD/tests/nucleo_f070rb/GCC_ARM/TESTS/mbedmicro-rtos-mbed/malloc/malloc.map -t GCC_ARM

befor refactoring
Total Static RAM memory (data + bss): 10172 bytes

after refactoring
Total Static RAM memory (data + bss): 10652 bytes

diff 
Total Static RAM memory (data + bss): 480 bytes
extern uint32_t mbed_heap_size;
printf("mbed_heap_size: %lu  \r\n", mbed_heap_size);

befor refactoring
mbed_heap_size: 4996

after refactoring
mbed_heap_size: 4516

diff 
mbed_heap_size: 480

@adbridge
Copy link
Contributor

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 24, 2017

Build : FAILURE

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

@studavekar
Copy link
Contributor

Looks git clone failure, Saw similar issue on few other PR as well

Receiving objects:  34% (11751/33717), 23.94 MiB | 39.00 KiB/s   
Receiving objects:  34% (11752/33717), 24.00 MiB | 41.00 KiB/s   
Receiving objects:  34% (11753/33717), 24.09 MiB | 46.00 KiB/s   
fatal: The remote end hung up unexpectedly
00:10:06.027 fatal: early EOF
00:10:06.027 fatal: index-pack failed
00:10:06.027 
00:10:06.027 	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1924)
00:10:06.027 	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIIm

@studavekar
Copy link
Contributor

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 24, 2017

Build : FAILURE

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

@adbridge
Copy link
Contributor

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 26, 2017

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Oct 26, 2017

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 26, 2017

@maciejbocianski NRF51_DK-IAR.tests-mbedmicro-rtos-mbed-malloc.Test multithreaded allocations failure in the last run - timeout with the error: [1509024079.73][CONN][RXD] Thread 0 error -11: Unknown

@maciejbocianski
Copy link
Contributor Author

problem fixed - there was lack of thread stack alignment

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 27, 2017

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 27, 2017

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2017

@maciejbocianski Looking at the commits here. Bad rebase? As I see here changes that went to master previously, and not related to this patch. Please review

@maciejbocianski
Copy link
Contributor Author

@0xc0170 done

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2017

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2017

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2017

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2017

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 16, 2017

Manually triggered CI uvisor job, should complete soon. This is the last CI that needs t o be completed for this patch.

Once done, ready for integration

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 16, 2017

@studavekar Waiting for nucleo_f070rb run

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 16, 2017

@studavekar Waiting for nucleo_f070rb run

@maciejbocianski As you asked about that nucleo, I could not identify any test run for this device, is that correct?

@maciejbocianski
Copy link
Contributor Author

maciejbocianski commented Nov 17, 2017

@studavekar @0xc0170
My fault, I fancied that nucleo_f070rb was connected to CI earlier but wasn't
BTW are we plan to add more devices to CI, especially devices with small RAM (<= 16kB)

@theotherjimmy
Copy link
Contributor

/morph uvisor-test

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 22, 2017

uvisor restarted, jenkins exception in the job

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

Successfully merging this pull request may close these issues.

9 participants