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

core: Replace epoch deadline with yield #1684

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

core: Replace epoch deadline with yield #1684

wants to merge 1 commit into from

Conversation

lann
Copy link
Collaborator

@lann lann commented Aug 9, 2023

With async yielding, instance execution can be implemented by dropping the async call future, with e.g. tokio::time::timeout.

/// details of the system's thread scheduler.
///
/// See [`wasmtime::Store::set_epoch_deadline`](https://docs.rs/wasmtime/latest/wasmtime/struct.Store.html#method.set_epoch_deadline).
pub fn set_deadline(&mut self, deadline: Instant) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Based on some searching I'm reasonably confident that this method isn't being used anywhere (public).

@@ -37,7 +37,7 @@ pub use io::OutputBuffer;
pub use store::{Store, StoreBuilder, Wasi, WasiVersion};

/// The default [`EngineBuilder::epoch_tick_interval`].
pub const DEFAULT_EPOCH_TICK_INTERVAL: Duration = Duration::from_millis(10);
pub const DEFAULT_EPOCH_TICK_INTERVAL: Duration = Duration::from_micros(100);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This arbitrarily updates the original arbitrary value, which "felt too slow" to me. 🤷

@rylev
Copy link
Collaborator

rylev commented Sep 21, 2023

@lann what's the state of this? I thought this had already been merged.

@lann
Copy link
Collaborator Author

lann commented Sep 21, 2023

Uhh I think I lost track of this back when e2e tests were broken for a while. Will refresh.

@rylev
Copy link
Collaborator

rylev commented Oct 18, 2023

@lann did this once again go stale?

@lann
Copy link
Collaborator Author

lann commented Oct 18, 2023

Yep, but it could still be reviewed and then I could just merge it after the next rebase 🙂

@lann lann requested a review from dicej October 18, 2023 13:09
With async yielding, instance execution can be implemented by dropping
the async call future, with e.g. tokio::time::timeout.

Signed-off-by: Lann Martin <[email protected]>
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.

2 participants