modified poll_oneoff to make it interruptible #1951
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hritikgupta
commented
Feb 10, 2023
hritikgupta
force-pushed
the
polloneoff
branch
from
f416f09
to
54ff77e
Compare
loganek
suggested changes
Feb 10, 2023
wenyongh
reviewed
Feb 13, 2023
hritikgupta
force-pushed
the
polloneoff
branch
2 times, most recently
from
6516265
to
b69d3f2
Compare
loganek
reviewed
Feb 13, 2023
hritikgupta
force-pushed
the
polloneoff
branch
2 times, most recently
from
0b790e0
to
bf8e2ea
Compare
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
loganek
reviewed
Feb 13, 2023
hritikgupta
force-pushed
the
polloneoff
branch
2 times, most recently
from
84b1ba8
to
ce45358
Compare
wenyongh
reviewed
Feb 14, 2023
hritikgupta
force-pushed
the
polloneoff
branch
2 times, most recently
from
b86c025
to
101e2f1
Compare
loganek
reviewed
Feb 14, 2023
loganek
reviewed
Feb 14, 2023
loganek
reviewed
Feb 14, 2023
loganek
reviewed
Feb 14, 2023
hritikgupta
force-pushed
the
polloneoff
branch
2 times, most recently
from
a6d704f
to
477692c
Compare
wenyongh
reviewed
Feb 15, 2023
loganek
reviewed
Feb 16, 2023
loganek
reviewed
Feb 16, 2023
loganek
reviewed
Feb 16, 2023
loganek
approved these changes
Feb 16, 2023
|
@wenyongh can we merge |
hritikgupta
force-pushed
the
polloneoff
branch
from
0546327
to
0b6f812
Compare
Sure, will do that soon. |
wenyongh
reviewed
Feb 16, 2023
| size_t *nevents, uint32 *nevents_app) | ||
| { | ||
| if (nsubscriptions == 0) | ||
| return 0; |
There was a problem hiding this comment.
Had better set *nevents_app = 0 before return success.
There was a problem hiding this comment.
wouldn't that anyway happen on L1077, given we have now initialised nevents to 0?
There was a problem hiding this comment.
Oh, that's really an issue, not sure why the prototype of execute_interruptible_poll_oneoff isn't same as wasmtime_ssp_poll_oneoff. My understanding is that we should define them as same prototype.
There was a problem hiding this comment.
it's mostly the same (that's what my intent was too) except for two arguments i.e. module_inst which needs to be passed to check if there are any exceptions that occurred & nevents_app
There was a problem hiding this comment.
Yes, adding module_inst is OK, but how about removing nevents_app? It is duplicated set inside execute_interruptible_poll_oneoff and after calling it.
There was a problem hiding this comment.
right, removed nevents_app
hritikgupta
force-pushed
the
polloneoff
branch
from
0b6f812
to
2e0a707
Compare
wenyongh
reviewed
Feb 16, 2023
hritikgupta
force-pushed
the
polloneoff
branch
from
2e0a707
to
5832c54
Compare
loganek
approved these changes
Feb 16, 2023
This was referenced Feb 20, 2023
wenyongh
pushed a commit
that referenced
this pull request
Feb 23, 2023
…1980) Fix issues in the libc-wasi `poll_oneoff` when thread manager is enabled: - The exception of a thread may be cleared when other thread runs into `proc_exit` and then calls `clear_wasi_proc_exit_exception`, so should not use `wasm_runtime_get_exception` to check whether an exception was thrown, use `wasm_cluster_is_thread_terminated` instead - We divided one time poll_oneoff into many times poll_oneoff to check the exception to avoid long time waiting in previous PR, but if all events returned by one time poll are all waiting events, we need to continue to wait but not return directly. Follow-up on #1951. Tested with multiple timeout values, with and without interruption and measured the time spent sleeping.
yamt
added a commit
to yamt/wasm-micro-runtime
that referenced
this pull request
Jan 24, 2024
While we used a different approach for poll_oneoff [1], the implementation works only when the poll list includes an absolute clock event. That is, if we have a thread which is polling on descriptors without a timeout, we fail to terminate the thread. This commit fixes it by applying wasm_runtime_begin_blocking_op to poll as well. [1] bytecodealliance#1951
yamt
added a commit
to yamt/wasm-micro-runtime
that referenced
this pull request
Jan 24, 2024
While we used a different approach for poll_oneoff [1], the implementation works only when the poll list includes an absolute clock event. That is, if we have a thread which is polling on descriptors without a timeout, we fail to terminate the thread. This commit fixes it by applying wasm_runtime_begin_blocking_op to poll as well. [1] bytecodealliance#1951
yamt
added a commit
to yamt/wasm-micro-runtime
that referenced
this pull request
Jan 24, 2024
While we used a different approach for poll_oneoff [1], the implementation works only when the poll list includes an absolute clock event. That is, if we have a thread which is polling on descriptors without a timeout, we fail to terminate the thread. This commit fixes it by applying wasm_runtime_begin_blocking_op to poll as well. [1] bytecodealliance#1951
yamt
added a commit
to yamt/wasm-micro-runtime
that referenced
this pull request
Jan 24, 2024
While we used a different approach for poll_oneoff [1], the implementation works only when the poll list includes an absolute clock event. That is, if we have a thread which is polling on descriptors without a timeout, we fail to terminate the thread. This commit fixes it by applying wasm_runtime_begin_blocking_op to poll as well. [1] bytecodealliance#1951
wenyongh
pushed a commit
that referenced
this pull request
Jan 25, 2024
While we used a different approach for poll_oneoff [1], the implementation works only when the poll list includes an absolute clock event. That is, if we have a thread which is polling on descriptors without a timeout, we fail to terminate the thread. This commit fixes it by applying wasm_runtime_begin_blocking_op to poll as well. [1] #1951
victoryang00
pushed a commit
to victoryang00/wamr-aot-gc-checkpoint-restore
that referenced
this pull request
May 27, 2024
victoryang00
pushed a commit
to victoryang00/wamr-aot-gc-checkpoint-restore
that referenced
this pull request
May 27, 2024
…ytecodealliance#1980) Fix issues in the libc-wasi `poll_oneoff` when thread manager is enabled: - The exception of a thread may be cleared when other thread runs into `proc_exit` and then calls `clear_wasi_proc_exit_exception`, so should not use `wasm_runtime_get_exception` to check whether an exception was thrown, use `wasm_cluster_is_thread_terminated` instead - We divided one time poll_oneoff into many times poll_oneoff to check the exception to avoid long time waiting in previous PR, but if all events returned by one time poll are all waiting events, we need to continue to wait but not return directly. Follow-up on bytecodealliance#1951. Tested with multiple timeout values, with and without interruption and measured the time spent sleeping.
victoryang00
pushed a commit
to victoryang00/wamr-aot-gc-checkpoint-restore
that referenced
this pull request
May 27, 2024
…lliance#3080) While we used a different approach for poll_oneoff [1], the implementation works only when the poll list includes an absolute clock event. That is, if we have a thread which is polling on descriptors without a timeout, we fail to terminate the thread. This commit fixes it by applying wasm_runtime_begin_blocking_op to poll as well. [1] bytecodealliance#1951
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.