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

Revert "Adjusting Stack size Allocation (IAR, LPC176x)" #5037

Merged
merged 1 commit into from Sep 7, 2017

Conversation

Projects
None yet
4 participants
@0xc0170
Member

0xc0170 commented Sep 6, 2017

This reverts commit fce2ca2. With the current heap for IAR (no dynamic yet), 2 tcp/udp tests fail with not enough heap memory.

@hasnainvirk @kjbracey-arm @geky

parallel tests fail, they allocate on heap Echo class

    // Startup echo threads in parallel
    for (unsigned int i = 0; i < MBED_CFG_UDP_CLIENT_ECHO_THREADS; i++) {
        echoers[i] = new Echo;
        echoers[i]->start(i, uuid);
    }

Should Arch PRO decrease any config there in those tests ,as not having enough heap ? With previous heap, there was not enough memory left for cellullar application, with the current our 2 tests fail due to not enough heap. I do not see the requirements for parallel testing - how much heap these test require. I can spot there 2x buffers 64bytes plus other objects, multiplied by 3, makes t he test to require lets assume something below 1k of heap... the target has at the moment 7k.

@0xc0170 0xc0170 added the needs: CI label Sep 6, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 6, 2017

/morph test-nightly

@geky

This comment has been minimized.

Member

geky commented Sep 6, 2017

We may be able to decrease the thread stack size for the threads.

Other than that, I'm not sure, we don't have a way for tests to require memory sizes. At best we can use the increasingly popular "MBED_EXTENDED_TESTS" define to skip the parallel tests by default, although this is the only test that is checking that the network stack is thread-safe.

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 6, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1230

All builds and test passed!

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

Merging. Unblocking mbed TLS update testing.

@theotherjimmy theotherjimmy merged commit 22b183a into ARMmbed:master Sep 7, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test-nightly Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 7, 2017

Other than that, I'm not sure, we don't have a way for tests to require memory sizes. At best we can use the increasingly popular "MBED_EXTENDED_TESTS" define to skip the parallel tests by default, although this is the only test that is checking that the network stack is thread-safe.

OK, I'll talk to @hasnainvirk once he is back, as this revert might affect cellular example (updating to IAR 8.x will fix it so soon enough !)

@0xc0170 0xc0170 deleted the 0xc0170:fix_arch_pro_heap branch Sep 7, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

@0xc0170 I'm hoping that the "breakage period" will be short. We should be able to minimize it.

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