Skip to content

Limiting the thread stack for parallel threads test #3484

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
Dec 23, 2016

Conversation

bridadan
Copy link
Contributor

@bridadan bridadan commented Dec 20, 2016

Description

Previously, the RTOS threads test would conditionally change the thread stack size for all test cases based on the target. Now, it uses the default stack size for all targets when threads are created serially, and uses a 512 byte stack for the threads that are created in parallel.

Also, this should remove the last failing test case on the ARCH_PRO platform :)

Status

READY

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

NO

Todos

  • Tests

Notes to Reviewers

Requesting reviews by @c1728p9 and @geky since they are familiar with all things RTOS and stack-sizey (yes that's a technical term)

Also requesting a review by @pan- as this may affect some of the BLE platforms/constrained mbeds.

Previously, the RTOS threads test was conditionally change the thread
stack size for all test cases based on the target. Now, it uses the
default stack size for all targets when threads are created serially,
and uses a 512 byte stack for the threads that are created in parallel.
@bridadan
Copy link
Contributor Author

/morph test

@geky
Copy link
Contributor

geky commented Dec 21, 2016

+1 for technical terminology

@mbed-bot
Copy link

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 1303

All builds and test passed!

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.

Looks good to me. Now we need to do this with the rest of the codebase 😄

Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

Looks good, and it apparently didn't break on any platform.

@bulislaw
Copy link
Member

I fixed it in features_cmsis5 branch, but using default stack size (4k by default but can be change per platform).

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 21, 2016

@VeliMattiLahtela Please restart CI ? looks like it;s not progressing.

@tommikas
Copy link
Contributor

continuous-integration/jenkins/pr-head is green. For some reason the status here isn't getting updated.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 21, 2016

continuous-integration/jenkins/pr-head is green. For some reason the status here isn't getting updated.

Needs to be green :-) I restarted 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.

8 participants