diff --git a/Cargo.lock b/Cargo.lock index 4b2fdb4..541f406 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -79,17 +79,39 @@ dependencies = [ "log", ] +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "fuchsia-cprng" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" + +[[package]] +name = "indexmap" +version = "1.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1885e79c1fc4b10f0e172c475f458b7f7b93061064d98c3293e98c5ba0c8b399" +dependencies = [ + "autocfg 1.1.0", + "hashbrown", +] + [[package]] name = "lazy_static" -version = "1.3.0" +version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc5729f27f159ddd61f4df6228e827e86643d4d3e7c32183cb30a1c08f604a14" +checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" @@ -131,6 +153,22 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b3a5d7cc97d6d30d8b9bc8fa19bf45349ffe46241e8816f50f62f6d6aaabee1" +[[package]] +name = "once_cell" +version = "1.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" + +[[package]] +name = "priority-queue" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7685ca4cc0b3ad748c22ce6803e23b55b9206ef7715b965ebeaf41639238fdc" +dependencies = [ + "autocfg 1.1.0", + "indexmap", +] + [[package]] name = "proc-macro2" version = "0.4.30" @@ -217,7 +255,10 @@ name = "samp-precise-timers" version = "1.0.0" dependencies = [ "fern", + "fnv", "log", + "once_cell", + "priority-queue", "samp", "slab", ] diff --git a/Cargo.toml b/Cargo.toml index 5604014..165a4e4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,9 @@ samp = { git = "https://github.com/ZOTTCE/samp-rs.git", rev = "2e9192713bf9a77ee slab = "0.4.2" log = "0.4.6" fern = "0.5.9" +priority-queue = "1.3.0" +once_cell = "1.19.0" +fnv = "1.0.7" [profile.release] -lto = true \ No newline at end of file +lto = true diff --git a/src/lib.rs b/src/lib.rs index 4ab20f1..b3d2696 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,21 +1,51 @@ #![warn(clippy::pedantic)] use amx_arguments::VariadicAmxArguments; -use log::info; +use log::{error, info}; +use priority_queue::PriorityQueue; use samp::amx::{Amx, AmxIdent}; use samp::cell::AmxString; use samp::error::{AmxError, AmxResult}; use samp::plugin::SampPlugin; use slab::Slab; +use std::cell::RefCell; +use std::cmp::Reverse; use std::convert::TryFrom; use std::time::{Duration, Instant}; -use timer::{Timer, TimerStaus}; - +use timer::Timer; mod amx_arguments; mod timer; -/// The plugin and its data: a list of scheduled timers -struct PreciseTimers { - timers: Slab, +thread_local! { + static TIMERS: RefCell> = RefCell::new(Slab::with_capacity(1000)); + static QUEUE: RefCell, fnv::FnvBuildHasher>> = RefCell::new(PriorityQueue::with_capacity_and_default_hasher(1000)); +} + +/// The plugin +struct PreciseTimers; + +#[derive(Clone)] +struct TimerScheduling { + interval: Option, + execution_forbidden: bool, + next_trigger: Instant, +} + +impl PartialEq for TimerScheduling { + fn eq(&self, other: &Self) -> bool { + self.next_trigger == other.next_trigger + } +} +impl Eq for TimerScheduling {} + +impl PartialOrd for TimerScheduling { + fn partial_cmp(&self, other: &Self) -> Option { + self.next_trigger.partial_cmp(&other.next_trigger) + } +} +impl Ord for TimerScheduling { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.next_trigger.cmp(&other.next_trigger) + } } impl PreciseTimers { @@ -38,18 +68,26 @@ impl PreciseTimers { // Find the callback by name and save its index let amx_callback_index = amx.find_public(&callback_name.to_string())?; + let next_trigger = Instant::now() + interval; let timer = Timer { - next_trigger: Instant::now() + interval, - interval: if repeat { Some(interval) } else { None }, passed_arguments, amx_identifier: AmxIdent::from(amx.amx().as_ptr()), amx_callback_index, - scheduled_for_removal: false, }; // Add the timer to the list. This is safe for Slab::retain() even if SetPreciseTimer was called from a timer's callback. - let key: usize = self.timers.insert(timer); + let key: usize = TIMERS.with_borrow_mut(|t| t.insert(timer)); + QUEUE.with_borrow_mut(|q| { + q.push( + key, + Reverse(TimerScheduling { + next_trigger, + interval: if repeat { Some(interval) } else { None }, + execution_forbidden: false, + }), + ) + }); // The timer's slot in Slab<> incresed by 1, so that 0 signifies an invalid timer in PAWN let timer_number = key .checked_add(1) @@ -66,10 +104,15 @@ impl PreciseTimers { #[allow(clippy::unnecessary_wraps)] #[samp::native(name = "DeletePreciseTimer")] pub fn delete(&mut self, _: &Amx, timer_number: usize) -> AmxResult { - // Subtract 1 from the passed timer_number (where 0=invalid) to get the actual Slab<> slot - if let Some(timer) = self.timers.get_mut(timer_number - 1) { - // We defer the removal so that we don't mess up the process_tick()->retain() iterator. - timer.scheduled_for_removal = true; + let key = timer_number - 1; + if QUEUE + .try_with(|q| { + q.borrow_mut().change_priority_by(&key, |scheduling| { + scheduling.0.execution_forbidden = true; + }) + }) + .map_err(|_| AmxError::MemoryAccess)? + { Ok(1) } else { Ok(0) @@ -89,13 +132,25 @@ impl PreciseTimers { interval: i32, repeat: bool, ) -> AmxResult { - if let Some(timer) = self.timers.get_mut(timer_number - 1) { - let interval = u64::try_from(interval) - .map(Duration::from_millis) - .or(Err(AmxError::Params))?; + let key = timer_number - 1; + let interval = u64::try_from(interval) + .map(Duration::from_millis) + .or(Err(AmxError::Params))?; - timer.next_trigger = Instant::now() + interval; - timer.interval = if repeat { Some(interval) } else { None }; + if QUEUE + .try_with(|q| { + q.borrow_mut().change_priority( + &key, + Reverse(TimerScheduling { + next_trigger: Instant::now() + interval, + interval: if repeat { Some(interval) } else { None }, + execution_forbidden: false, + }), + ) + }) + .map_err(|_| AmxError::MemoryAccess)? + .is_some() + { Ok(1) } else { Ok(0) @@ -105,7 +160,7 @@ impl PreciseTimers { impl SampPlugin for PreciseTimers { fn on_load(&mut self) { - info!("net4game.com/samp-precise-timers by Brian Misiak loaded correctly."); + info!("samp-precise-timers 3 by Brian Misiak loaded correctly."); } #[allow(clippy::inline_always)] @@ -114,18 +169,77 @@ impl SampPlugin for PreciseTimers { // Rust's Instant is monotonic and nondecreasing, even during NTP time adjustment. let now = Instant::now(); - // 💀⚠ Because of FFI with C, Rust can't notice the simultaneous mutation of self.timers, but the iterator could get messed up in case of - // Slab::retain() -> Timer::trigger() -> PAWN callback/ffi which calls DeletePreciseTimer() -> Slab::remove. - // That's why the DeletePreciseTimer() schedules timers for deletion instead of doing it right away. - // Slab::retain() is, however, okay with inserting new timers during its execution, even in case of reallocation when over capacity. - self.timers.retain(|_key: usize, timer| { - timer.trigger_if_due(now) == TimerStaus::MightTriggerInTheFuture - }); + loop { + let triggered_timer = QUEUE.with_borrow(|q| match q.peek() { + Some(( + &key, + &Reverse(TimerScheduling { + next_trigger, + interval, + execution_forbidden, + }), + )) if next_trigger <= now => Some((key, interval, execution_forbidden)), + _ => None, + }); + let Some((key, interval, execution_forbidden)) = triggered_timer else { + break; + }; + + if let (Some(interval), false) = (interval, execution_forbidden) { + let next_trigger = now + interval; + QUEUE.with_borrow_mut(|q| { + q.change_priority( + &key, + Reverse(TimerScheduling { + next_trigger, + execution_forbidden, + interval: Some(interval), + }), + ) + .expect("failed to reschedule repeating timer"); + }); + + TIMERS.with_borrow_mut(|t| { + let timer = t.get_mut(key).expect("slab should contain repeating timer"); + + if let Err(err) = timer.execute_pawn_callback() { + error!("Error executing repeating timer callback: {}", err); + } + }); + } else { + // Must pop before the timer is executed, so that + // it can't schedule anything as the very next timer before + // we have a chance to pop from the queue. + let (popped_key, _) = QUEUE + .with_borrow_mut(|q| q.pop()) + .expect("priority queue should have at least the timer we peeked"); + assert_eq!(popped_key, key); + let mut removed_timer = TIMERS.with_borrow_mut(|t| t.remove(key)); + + if !execution_forbidden { + if let Err(err) = removed_timer.execute_pawn_callback() { + error!("Error executing non-repeating timer callback: {}", err); + } + } + } + } } fn on_amx_unload(&mut self, unloaded_amx: &Amx) { - self.timers - .retain(|_, timer| !timer.was_scheduled_by_amx(unloaded_amx)); + let mut removed_timers = Vec::new(); + TIMERS.with_borrow_mut(|t| { + t.retain(|key, timer| { + if timer.was_scheduled_by_amx(unloaded_amx) { + removed_timers.push(key); + false + } else { + true + } + }) + }); + for key in removed_timers { + QUEUE.with_borrow_mut(|q| q.remove(&key)); + } } } @@ -148,8 +262,6 @@ samp::initialize_plugin!( .chain(samp_logger) .apply(); - PreciseTimers { - timers: Slab::with_capacity(1000) - } + PreciseTimers } ); diff --git a/src/timer.rs b/src/timer.rs index 7231cb7..5e9f8bb 100644 --- a/src/timer.rs +++ b/src/timer.rs @@ -1,27 +1,13 @@ use crate::amx_arguments::VariadicAmxArguments; -use log::error; -use samp::{ - amx::AmxIdent, - error::AmxError, - prelude::AmxResult, -}; -use std::time::{Duration, Instant}; -#[derive(PartialEq)] -pub(crate) enum TimerStaus { - MightTriggerInTheFuture, - WillNeverTriggerAgain, -} +use samp::{amx::AmxIdent, error::AmxError, prelude::AmxResult}; /// The Timer struct represents a single scheduled timer #[derive(Debug, Clone)] pub(crate) struct Timer { - pub next_trigger: Instant, - pub interval: Option, pub passed_arguments: VariadicAmxArguments, pub amx_identifier: AmxIdent, pub amx_callback_index: samp::consts::AmxExecIdx, - pub scheduled_for_removal: bool, } impl Timer { @@ -30,7 +16,7 @@ impl Timer { } /// This function executes the callback provided to the `SetPreciseTimer` native. - pub fn trigger(&mut self) -> AmxResult<()> { + pub fn execute_pawn_callback(&mut self) -> AmxResult<()> { // Get the AMX which scheduled the timer let amx = samp::amx::get(self.amx_identifier).ok_or(AmxError::NotFound)?; let allocator = amx.allocator(); @@ -42,41 +28,4 @@ impl Timer { Ok(()) } - - /// Checks if it's time to trigger the timer yet. If so, triggers it. - /// Returns info about whether the timer is okay to remove now - #[inline(always)] - pub fn trigger_if_due(&mut self, now: Instant) -> TimerStaus { - use TimerStaus::{MightTriggerInTheFuture, WillNeverTriggerAgain}; - - if self.scheduled_for_removal { - // Ordered removed. Do not execute the timer's callback. - return WillNeverTriggerAgain; - } - if self.next_trigger > now { - // Not the time to trigger it yet. - return MightTriggerInTheFuture; - } - - // Execute the callback: - if let Err(err) = self.trigger() { - error!("Error executing timer callback: {}", err); - } - - if let Some(interval) = self.interval { - self.next_trigger = now + interval; - // It repeats. Keep it, unless removed from PAWN when it was triggered just now. - // Hopfully LLVM doesn't elide this check, but it could, given that we checked - // scheduled_for_removal earlier, .trigger() doesn't modify it, and Amx::exec - // is wrongly marked safe despite its potential for aliased references. - if self.scheduled_for_removal { - WillNeverTriggerAgain - } else { - MightTriggerInTheFuture - } - } else { - // Remove the timer. It got triggered and does not repeat - WillNeverTriggerAgain - } - } }