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

Simplify the timers API #507

Merged
merged 1 commit into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions wasm-node/javascript/src/instance/bindings-smoldot-light.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ export default function (config: Config): { imports: WebAssembly.ModuleImports,
},

// Must call `timer_finished` after the given number of milliseconds has elapsed.
start_timer: (id: number, ms: number) => {
start_timer: (ms: number) => {
if (killedTracked.killed) return;

const instance = config.instance!;
Expand All @@ -327,14 +327,14 @@ export default function (config: Config): { imports: WebAssembly.ModuleImports,
setImmediate(() => {
if (killedTracked.killed) return;
try {
instance.exports.timer_finished(id);
instance.exports.timer_finished();
} catch (_error) { }
})
} else {
setTimeout(() => {
if (killedTracked.killed) return;
try {
instance.exports.timer_finished(id);
instance.exports.timer_finished();
} catch (_error) { }
}, ms)
}
Expand Down
2 changes: 1 addition & 1 deletion wasm-node/javascript/src/instance/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface SmoldotWasmExports extends WebAssembly.Exports {
json_rpc_send: (textBufferIndex: number, chainId: number) => number,
json_rpc_responses_peek: (chainId: number) => number,
json_rpc_responses_pop: (chainId: number) => void,
timer_finished: (timerId: number) => void,
timer_finished: () => void,
connection_open_single_stream: (connectionId: number, handshakeTy: number, initialWritableBytes: number, writeClosable: number) => void,
connection_open_multi_stream: (connectionId: number, handshakeTyBufferIndex: number) => void,
stream_writable_bytes: (connectionId: number, streamId: number, numBytes: number) => void,
Expand Down
9 changes: 4 additions & 5 deletions wasm-node/rust/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ extern "C" {
/// [`advance_execution`] should be called again immediately after it returns.
pub fn advance_execution_ready();

/// After at least `milliseconds` milliseconds have passed, must call [`timer_finished`] with
/// the `id` passed as parameter.
/// After at least `milliseconds` milliseconds have passed, [`timer_finished`] must be called.
///
/// It is not a logic error to call [`timer_finished`] *before* `milliseconds` milliseconds
/// have passed, and this will likely cause smoldot to restart a new timer for the remainder
Expand All @@ -147,7 +146,7 @@ extern "C" {
/// If `milliseconds` is 0, [`timer_finished`] should be called as soon as possible.
///
/// `milliseconds` never contains a negative number, `NaN` or infinite.
pub fn start_timer(id: u32, milliseconds: f64);
pub fn start_timer(milliseconds: f64);

/// Must initialize a new connection that tries to connect to the given multiaddress.
///
Expand Down Expand Up @@ -481,8 +480,8 @@ pub extern "C" fn json_rpc_responses_pop(chain_id: u32) {

/// Must be called in response to [`start_timer`] after the given duration has passed.
#[no_mangle]
pub extern "C" fn timer_finished(timer_id: u32) {
crate::timers::timer_finished(timer_id);
pub extern "C" fn timer_finished() {
crate::timers::timer_finished();
}

/// Called by the JavaScript code if the connection switches to the `Open` state. The connection
Expand Down
12 changes: 1 addition & 11 deletions wasm-node/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#![deny(rustdoc::broken_intra_doc_links)]
#![deny(unused_crate_dependencies)]

use core::{future, mem, num::NonZeroU32, pin::Pin, str, time::Duration};
use core::{future, mem, num::NonZeroU32, pin::Pin, str};
use futures_util::{stream, Stream as _, StreamExt as _};
use smoldot_light::HandleRpcError;
use std::{
Expand All @@ -35,16 +35,6 @@ mod init;
mod platform;
mod timers;

/// Uses the environment to invoke `closure` after at least `duration` has elapsed.
fn start_timer_wrap(duration: Duration, closure: impl FnOnce() + 'static) {
let callback: Box<Box<dyn FnOnce() + 'static>> = Box::new(Box::new(closure));
let timer_id = u32::try_from(Box::into_raw(callback) as usize).unwrap();
// Note that ideally `duration` should be rounded up in order to make sure that it is not
// truncated, but the precision of an `f64` is so high and the precision of the operating
// system generally so low that this is not worth dealing with.
unsafe { bindings::start_timer(timer_id, duration.as_secs_f64() * 1000.0) }
}

static CLIENT: Mutex<Option<init::Client<platform::Platform, ()>>> = Mutex::new(None);

fn init(max_log_level: u32, enable_current_task: u32) {
Expand Down
25 changes: 15 additions & 10 deletions wasm-node/rust/src/timers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
//! is only used in order to wake up when the earliest timer finishes, then restarted for the next
//! timer.

use crate::bindings;

use core::{
cmp::{Eq, Ord, Ordering, PartialEq, PartialOrd},
future,
Expand All @@ -31,13 +33,8 @@ use core::{
};
use std::{collections::BinaryHeap, sync::Mutex, time::Instant};

pub(crate) fn timer_finished(timer_id: u32) {
let callback = {
let ptr = timer_id as *mut Box<dyn FnOnce() + 'static>;
unsafe { Box::from_raw(ptr) }
};

callback();
pub(crate) fn timer_finished() {
process_timers();
}

/// `Future` that automatically wakes up after a certain amount of time has elapsed.
Expand Down Expand Up @@ -81,9 +78,9 @@ impl Delay {
// If the timer that has just been inserted is the one that ends the soonest, then
// actually start the callback that will process timers.
// Ideally we would instead cancel or update the deadline of the previous call to
// `start_timer_wrap`, but this isn't possible.
// `start_timer`, but this isn't possible.
if lock.timers_queue.peek().unwrap().timer_id == timer_id {
super::start_timer_wrap(when - now, process_timers);
start_timer(when - now);
}

Delay {
Expand Down Expand Up @@ -245,10 +242,18 @@ fn process_timers() {
};

if let Some(next_wakeup) = next_wakeup {
super::start_timer_wrap(lock.time_zero + next_wakeup - now, process_timers);
start_timer(lock.time_zero + next_wakeup - now);
} else {
// Clean up memory a bit. Hopefully this doesn't impact performances too much.
lock.timers_queue.shrink_to_fit();
lock.timers.shrink_to_fit();
}
}

/// Instructs the environment to call [`process_timers`] after the given duration.
fn start_timer(duration: Duration) {
// Note that ideally `duration` should be rounded up in order to make sure that it is not
// truncated, but the precision of an `f64` is so high and the precision of the operating
// system generally so low that this is not worth dealing with.
unsafe { bindings::start_timer(duration.as_secs_f64() * 1000.0) }
}