From f2bdaa3fedd9c9b16628295094fe39640fff7ffe Mon Sep 17 00:00:00 2001 From: Vitali Lovich Date: Wed, 1 May 2024 16:05:27 -0700 Subject: [PATCH] Doc cleanup (#660) * Cleanup some confusing wording for timers. * Fix cargo doc warnings --- glommio/src/io/dma_file.rs | 6 ++--- glommio/src/timer/mod.rs | 5 +++- glommio/src/timer/timer_impl.rs | 41 +++++++++++++++++++++++++-------- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/glommio/src/io/dma_file.rs b/glommio/src/io/dma_file.rs index 334f5bf2a..5e1f8532a 100644 --- a/glommio/src/io/dma_file.rs +++ b/glommio/src/io/dma_file.rs @@ -488,7 +488,7 @@ impl DmaFile { /// Copies a file range from one file to another in kernel space. This is going to have the same performance /// characteristic as splice except if both files are on the same filesystem and the filesystem supports reflinks. /// In that case, the underlying disk blocks will be CoW linked instead of actually performing a copy. - /// Since `copy_file_range` is not yet implemented on io_uring (https://github.com/axboe/liburing/issues/831), + /// Since `copy_file_range` is not yet implemented on io_uring , /// this is just a dispatch to the blocking thread pool to do the syscall. pub async fn copy_file_range_aligned( &self, @@ -643,7 +643,7 @@ impl DmaFile { } /// The major ID of the device containing the filesystem where the file resides. - /// The device may be found by issuing a `readlink`` on `/sys/dev/block/:` + /// The device may be found by issuing a `readlink` on `/sys/dev/block/:` pub fn dev_major(&self) -> u32 { self.file.dev_major } @@ -803,7 +803,7 @@ impl OwnedDmaFile { } /// The major ID of the device containing the filesystem where the file resides. - /// The device may be found by issuing a `readlink`` on `/sys/dev/block/:` + /// The device may be found by issuing a `readlink` on `/sys/dev/block/:` pub fn dev_major(&self) -> u32 { self.file.dev_major } diff --git a/glommio/src/timer/mod.rs b/glommio/src/timer/mod.rs index 90d917ba5..bb0b039f2 100644 --- a/glommio/src/timer/mod.rs +++ b/glommio/src/timer/mod.rs @@ -11,7 +11,10 @@ pub use timer_impl::{Timer, TimerActionOnce, TimerActionRepeat}; type Result = crate::Result; -/// Sleep for some time. +/// Sleep for some time on the current task. Explicit sleeps can introduce undesirable delays if not used correctly. +/// Consider using [crate::timer::timeout] instead if you are implementing timeout-like semantics or +/// [crate::timer::TimerActionOnce] if you need to schedule a future for some later date in the future without needing +/// to await. /// /// ``` /// use glommio::{timer::sleep, LocalExecutor}; diff --git a/glommio/src/timer/timer_impl.rs b/glommio/src/timer/timer_impl.rs index 819977184..909979b0d 100644 --- a/glommio/src/timer/timer_impl.rs +++ b/glommio/src/timer/timer_impl.rs @@ -51,11 +51,17 @@ impl Inner { /// A timer that expires after a duration of time. /// -/// Timers are futures that output the [`Instant`] at which they fired. -/// Note that because of that, Timers always block the current task queue -/// in which they currently execute. +/// Timers are futures that output the [`Instant`] at which they fired. Awaiting it causes execution of the current +/// task to be delayed by that amount which can be undesirable in an async framework to leverage full concurrency +/// of execution. /// -/// In most situations you will want to use [`TimerActionOnce`] +/// In most situations you will want to use [`TimerActionOnce`] which is a convenience wrapper around spawning a new +/// detached task on the executor into the current task queue [crate::spawn_local_into] that does a sleep before running +/// a provided future). Detaching is important so that the future is actually scheduled immediately without needing to +/// be awaited. +/// +/// If you want timeout-like semantics where a future has to complete within a given deadline, consider using +/// [crate::timer::timeout]. /// /// # Examples /// @@ -185,12 +191,29 @@ impl Future for Timer { /// The TimerActionOnce struct provides an ergonomic way to fire an action at a /// later point in time. /// -/// In practice [`Timer`] is hard to use because it will always block the -/// current task queue. This is rarely what one wants. +/// In practice [`Timer`] is hard to use because it will always delay the +/// current task it is awaited on. This is rarely what one wants to exploit the +/// concurrency promises of an async framework. /// /// The [`TimerActionOnce`] creates a timer in the background and executes an /// action when the timer expires. It also provides a convenient way to cancel a -/// timer. +/// timer. This is a convenience wrapper around equivalent code like this: +/// +/// ``` +/// use glommio::{timer::TimerActionOnce, LocalExecutorBuilder}; +/// use std::time::Duration; +/// +/// let handle = LocalExecutorBuilder::default() +/// .spawn(|| async move { +/// let task = glommio::spawn_local(async move { +/// glommio::timer::sleep(Duration::from_millis(100)).await; +/// println!("Executed once") +/// }); +/// task.detach().await; +/// }) +/// .unwrap(); +/// handle.join().unwrap(); +/// ``` /// /// [`Timer`]: struct.Timer.html #[derive(Debug)] @@ -554,7 +577,7 @@ impl TimerActionRepeat { /// /// * `action_gen` a Future to be executed repeatedly. The Future's return /// value must be - /// Option. If [`Some`], It will execute again after Duration + /// `Option`. If [`Some`], It will execute again after Duration /// elapses. If `None`, it stops. /// * `tq` the [`TaskQueueHandle`] for the TaskQueue we want. /// @@ -618,7 +641,7 @@ impl TimerActionRepeat { /// /// * `action_gen` a Future to be executed repeatedly. The Future's return /// value must be - /// Option. If [`Some`], It will execute again after Duration + /// `Option`. If [`Some`], It will execute again after Duration /// elapses. If `None`, it stops. /// /// # Examples