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

add initial stress test #2364

Merged
merged 1 commit into from
Aug 1, 2023
Merged

Conversation

Zzzabiyaka
Copy link
Contributor

@Zzzabiyaka Zzzabiyaka commented Jul 17, 2023

We need to make a test that runs longer than the tests we had before to check some problems that might happen after running for some time (e.g. memory corruption or something else)

@Zzzabiyaka Zzzabiyaka marked this pull request as draft July 17, 2023 12:54
@Zzzabiyaka Zzzabiyaka force-pushed the add_stress_test branch 4 times, most recently from 850f2f3 to 7f926ad Compare July 21, 2023 13:12
core/iwasm/libraries/lib-wasi-threads/test/build.sh Outdated Show resolved Hide resolved
core/iwasm/libraries/lib-wasi-threads/test/build.sh Outdated Show resolved Hide resolved
core/iwasm/libraries/lib-wasi-threads/test/build.sh Outdated Show resolved Hide resolved
int
main(int argc, char **argv)
{
for (; factorised_number < NUM_ITER; ++factorised_number) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note - if you try to maximize the number of threads running all the times, this code does not do that. Imagine situation where you have 3 threads (1, 2, 3) and for some reason 1 is really slow, but 2 and 3 are fast. Once 1, 2, and 3 are spawned, and then 2 and 3 complete, they'll have to wait for thread 1 to complete before 2 and 3 is spawned again. If you want to achieve maximum concurrency at most possible times, you should probably just maintain a list of thread ids and keep track on the number of active threads.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure we can benefit much from testing how many threads exactly run together. Looks like we're more interested in creation/destruction as a part of proposal, aren't we?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, and by implementing it the way it's currently done in this PR, we're delaying spawning new threads

@Zzzabiyaka Zzzabiyaka force-pushed the add_stress_test branch 5 times, most recently from 7f20e6b to 0a50623 Compare July 24, 2023 14:51
*/

#ifndef __wasi__
#error This example only compiles to WASM/WASI target
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we using any specific WASM/WASI instruction here?

Copy link
Contributor Author

@Zzzabiyaka Zzzabiyaka Jul 24, 2023

Choose a reason for hiding this comment

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

looks like we're not but I'm not sure we should allow anyone running this test without WASI define, should we?

I mean, I checked this code only with WASI sdk, where this flag is defined. Without it, you won't be able to run build.sh etc

Copy link
Collaborator

Choose a reason for hiding this comment

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

sure, all I'm saying that one can most likely compile the code with a regular C compiler on linux.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, probably they can.

@Zzzabiyaka Zzzabiyaka force-pushed the add_stress_test branch 7 times, most recently from 8f60126 to ef548af Compare July 25, 2023 15:37
@Zzzabiyaka Zzzabiyaka marked this pull request as ready for review July 25, 2023 15:52
check_if_prime(void *value)
{
unsigned int *num = (unsigned int *)(value);
usleep(10000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can probably make the sleep even smaller. I think for stress tests it'd be good to not only validate if the runtime can have multiple threads running at the same time for an extended period of time, but also whether multiple thread create/stop operations ran in a short amount of time work well. This can be implemented here, but perhaps we need another stress test for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I also feel like another test is good for it

core/iwasm/libraries/lib-wasi-threads/test/build.sh Outdated Show resolved Hide resolved
tests/wamr-test-suites/wasi-test-script/run_wasi_tests.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@loganek loganek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@Zzzabiyaka
Copy link
Contributor Author

Zzzabiyaka commented Jul 28, 2023

This PR is currently blocked on #2389

UPD: 2389 is merged, this PR is rebased and green

@wenyongh wenyongh mentioned this pull request Jul 30, 2023
@Zzzabiyaka Zzzabiyaka force-pushed the add_stress_test branch 3 times, most recently from 6377117 to b4b1261 Compare July 31, 2023 11:41
Copy link
Collaborator

@loganek loganek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@wenyongh wenyongh merged commit b88f2c0 into bytecodealliance:main Aug 1, 2023
611 checks passed
Zzzabiyaka added a commit to Zzzabiyaka/wasm-micro-runtime that referenced this pull request Aug 1, 2023
We need to make a test that runs longer than the tests we had before to check
some problems that might happen after running for some time (e.g. memory
corruption or something else).
@Zzzabiyaka Zzzabiyaka mentioned this pull request Aug 4, 2023
wenyongh pushed a commit that referenced this pull request Aug 17, 2023
Follows up #2364 where we discussed that we might want to have a test
which has really short thread function and creates many threads.
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
We need to make a test that runs longer than the tests we had before to check
some problems that might happen after running for some time (e.g. memory
corruption or something else).
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Follows up bytecodealliance#2364 where we discussed that we might want to have a test
which has really short thread function and creates many threads.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants