From ad223362451688c13a4441563210f58bdb046a78 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Wed, 17 May 2023 13:59:55 -0600 Subject: [PATCH] feat(ext/web): Request higher-resolution timer on Windows if user requests setTimeout w/short delay (#19149) If a timer is requested with <=100ms resolution, request the high-res timer. Since the default Windows timer period is 15ms, this means a 100ms timer could fire at 115ms (15% late). We assume that timers longer than 100ms are a reasonable cutoff here. The high-res timers on Windows are still limited. Unfortuntely this means that our shortest duration 4ms timers can still be 25% late, but without a more complex timer system or spinning on the clock itself, we're somewhat bounded by the OS' scheduler itself. --- Cargo.lock | 1 + Cargo.toml | 1 + ext/web/Cargo.toml | 1 + ext/web/hr_timer_lock.rs | 67 ++++++++++++++++++++++++++++++++++++++++ ext/web/lib.rs | 1 + ext/web/timers.rs | 18 ++++++++++- 6 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 ext/web/hr_timer_lock.rs diff --git a/Cargo.lock b/Cargo.lock index 19821855a4..5a1a5c4b35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1374,6 +1374,7 @@ dependencies = [ "serde", "tokio", "uuid", + "windows-sys 0.48.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index a45c2b2fd4..7e0d5083fc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -163,6 +163,7 @@ nix = "=0.24.2" fwdansi = "=1.1.0" winres = "=0.1.12" winapi = "=0.3.9" +windows-sys = { version = "0.48.0", features = ["Win32_Media"] } # NB: the `bench` and `release` profiles must remain EXACTLY the same. [profile.release] diff --git a/ext/web/Cargo.toml b/ext/web/Cargo.toml index 9ac7217f13..ebb602c5cd 100644 --- a/ext/web/Cargo.toml +++ b/ext/web/Cargo.toml @@ -22,6 +22,7 @@ flate2.workspace = true serde = "1.0.149" tokio.workspace = true uuid = { workspace = true, features = ["serde"] } +windows-sys.workspace = true [dev-dependencies] deno_bench_util.workspace = true diff --git a/ext/web/hr_timer_lock.rs b/ext/web/hr_timer_lock.rs new file mode 100644 index 0000000000..f1f588d6c8 --- /dev/null +++ b/ext/web/hr_timer_lock.rs @@ -0,0 +1,67 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +#[cfg(target_os = "windows")] +mod windows { + use std::marker::PhantomData; + use std::sync::atomic::AtomicU32; + + pub(crate) struct HrTimerLock { + pub(super) _unconstructable: PhantomData<()>, + } + + /// Decrease the reference count of the HR timer on drop. + impl Drop for HrTimerLock { + fn drop(&mut self) { + dec_ref(); + } + } + + /// Maintains the HR timer refcount. This should be more than sufficient as 2^32 timers would be + /// an impossible situation, and if it does somehow happen, the worst case is that we'll disable + /// the high-res timer when we shouldn't (and things would eventually return to proper operation). + static TIMER_REFCOUNT: AtomicU32 = AtomicU32::new(0); + + pub(super) fn inc_ref() { + let old = TIMER_REFCOUNT.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + // Overflow/underflow sanity check in debug mode + debug_assert!(old != u32::MAX); + if old == 0 { + lock_hr(); + } + } + + fn dec_ref() { + let old = TIMER_REFCOUNT.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); + // Overflow/underflow sanity check in debug mode + debug_assert!(old != 0); + if old == 1 { + unlock_hr(); + } + } + + /// If the refcount is > 0, we ask Windows for a lower timer period once. While the underlying + /// Windows timeBeginPeriod/timeEndPeriod API can manage its own reference counts, we choose to + /// use it once per process and avoid nesting these calls. + fn lock_hr() { + // SAFETY: We just want to set the timer period here + unsafe { windows_sys::Win32::Media::timeBeginPeriod(1) }; + } + + fn unlock_hr() { + // SAFETY: We just want to set the timer period here + unsafe { windows_sys::Win32::Media::timeEndPeriod(1) }; + } +} + +#[cfg(target_os = "windows")] +pub(crate) fn hr_timer_lock() -> windows::HrTimerLock { + windows::inc_ref(); + windows::HrTimerLock { + _unconstructable: Default::default(), + } +} + +/// No-op on other platforms. +#[cfg(not(target_os = "windows"))] +pub(crate) fn hr_timer_lock() -> (std::marker::PhantomData<()>,) { + (std::marker::PhantomData::default(),) +} diff --git a/ext/web/lib.rs b/ext/web/lib.rs index 3f4468f1f1..adbc9f2622 100644 --- a/ext/web/lib.rs +++ b/ext/web/lib.rs @@ -2,6 +2,7 @@ mod blob; mod compression; +mod hr_timer_lock; mod message_port; mod timers; diff --git a/ext/web/timers.rs b/ext/web/timers.rs index 252cd4ad41..54e185abda 100644 --- a/ext/web/timers.rs +++ b/ext/web/timers.rs @@ -2,9 +2,9 @@ //! This module helps deno implement timers and performance APIs. +use crate::hr_timer_lock::hr_timer_lock; use deno_core::error::AnyError; use deno_core::op; - use deno_core::CancelFuture; use deno_core::CancelHandle; use deno_core::OpState; @@ -86,8 +86,24 @@ pub async fn op_sleep( rid: ResourceId, ) -> Result { let handle = state.borrow().resource_table.get::(rid)?; + + // If a timer is requested with <=100ms resolution, request the high-res timer. Since the default + // Windows timer period is 15ms, this means a 100ms timer could fire at 115ms (15% late). We assume that + // timers longer than 100ms are a reasonable cutoff here. + + // The high-res timers on Windows are still limited. Unfortuntely this means that our shortest duration 4ms timers + // can still be 25% late, but without a more complex timer system or spinning on the clock itself, we're somewhat + // bounded by the OS' scheduler itself. + let _hr_timer_lock = if millis <= 100 { + Some(hr_timer_lock()) + } else { + None + }; + let res = tokio::time::sleep(Duration::from_millis(millis)) .or_cancel(handle.0.clone()) .await; + + // We release the high-res timer lock here, either by being cancelled or resolving. Ok(res.is_ok()) }